Go Down

Topic: name lookup of 'i' changed for new ISO 'for' scoping (Read 7724 times) previous topic - next topic

Nantonos


Effectively it is warning you about multiple redeclarations of "i".


I don't understand why that would be a problem; each one is declared in it's own scope and ceases to exist outside that scope. (Which is what caused the error, because of premature termination of one of the loops).

Nantonos

#31
Nov 05, 2012, 04:13 am Last Edit: Nov 05, 2012, 04:14 am by Nantonos Reason: 1

Anyone who writes commercial C code without a coding style and a tool that enforces that style (even if that tool is just the compiler with extra warnings enabled) is IMO stupid and irresponsible.



source: Abstruse Goose

dc42

Nice cartoon!



Effectively it is warning you about multiple redeclarations of "i".


I don't understand why that would be a problem; each one is declared in it's own scope and ceases to exist outside that scope. (Which is what caused the error, because of premature termination of one of the loops).


You are right, it's not a problem. It's also good programming style to declare each variable in the smallest scope possible, in particular to declare for-loop counter variables in the loop header.
Formal verification of safety-critical software, software development, and electronic design and prototyping. See http://www.eschertech.com. Please do not ask for unpaid help via PM, use the forum.

Nick Gammon


I don't understand why that would be a problem; each one is declared in it's own scope and ceases to exist outside that scope. (Which is what caused the error, because of premature termination of one of the loops).


I was wrong. In earlier compilers that (the multiple declarations) would have been a problem. PaulS picked up the real problem.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Nantonos

Nick, that is my (Pascal, Modula-2) background showing through. It never occurred to me that compilers would allow a variable to leak outside the scope in which it was declared. Still learning the joys of C/C++

Code: [Select]
self.foot(shoot)

Nick Gammon

Strictly speaking, I suppose, a for loop doesn't really have scope.

eg.

Code: [Select]

for (int i = 0; i < 10; i++)
  b++;


What is the scope? There is no block there. So in early versions of the compiler(s) that was taken as declaring a local variable (to the block in which that for loop resided) on-the-fly.

But then people (including me) would copy and paste, like this:

Code: [Select]

for (int i = 0; i < 10; i++)
  b++;

for (int i = 0; i < 10; i++)
  c++;


Then you got a message about duplicate declaration of "i" because it was declared on-the-fly twice.

So it looks like they changed (or clarified) the standard so that the declaration of "i" only persists for the statement (or block) that the for loop executes.

This is good in a way, but now you can't do this:

Code: [Select]

for (int i = 0; i < 10; i++)
if (strcmp (foo [i], "bar") == 0)
    break;

matching_index = i;


Because "i" is now out of scope. And you used to do that a fair bit as you iterated through a loop (eg. finding a match on a string) and then when you left the loop you still knew which value of "i" was the matching one.

Example sketch:

Code: [Select]

void setup ()
  {
  char * foo [3] = { "a", "bar", "c" };
 
  for (int i = 0; i < 3; i++)
    if (strcmp (foo [i], "bar") == 0)
      break;
 
  int matching_index = i;   
  }  // end of setup
 
void loop ()   {  }  // end of loop


Error:

Code: [Select]

sketch_nov06a.cpp: In function 'void setup()':
sketch_nov06a:8: error: name lookup of 'i' changed for new ISO 'for' scoping
sketch_nov06a:4: error: using obsolete binding at 'i'


Ach! Obsolete. Makes me feel old. ;)
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Nantonos


This is good in a way, but now you can't do this:

Code: [Select]

for (int i = 0; i < 10; i++)
if (strcmp (foo [i], "bar") == 0)
    break;

matching_index = i;


Because "i" is now out of scope.


True, but you can do this

Code: [Select]

int i;
for (i = 0; i < 10; i++)
if (strcmp (foo [i], "bar") == 0)
    break;

matching_index = i;


i.e. move the declaration out into the block where it is used.

To my eyes that is a while loop. Jumping out of a loop looks to me like something to avoid, only slightly better than jumping into one.

Nantonos

OK, now I feel slightly ill.The compiler has no problem with

Code: [Select]
void setup()

  Serial.begin(38400);
  int i = 0;
  i = i++ + ++i; // strewth!
  Serial.println(i);
}

void loop()
{
}

PaulS

Quote
The compiler has no problem with

Any why should it? There is nothing syntactically wrong with that code. Pointless, yes, but not wrong.


Nick Gammon

I tested that code and got 3.

However the result is (as you might expect), undefined:

http://www.stroustrup.com/bs_faq2.html#evaluation-order

Also see: http://en.wikipedia.org/wiki/Sequence_point

The value of i++ between sequence points is undefined (that is, it might, or might not, be incremented yet).
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Nantonos

#41
Nov 07, 2012, 08:35 am Last Edit: Nov 07, 2012, 08:37 am by Nantonos Reason: 1

I tested that code and got 3.

However the result is (as you might expect), undefined:


Yes, I had tested it as well. The point of my question was more about the lack of compiler errors or warnings about doing such an undefined operation.

And indeed, from the link you cited:
Quote
Compilers could warn about such examples, which are typically subtle bugs (or potential subtle bugs). I'm disappointed that after decades, most compilers still don't warn, leaving that job to specialized, separate, and underused tools.

dc42



I tested that code and got 3.

However the result is (as you might expect), undefined:


Yes, I had tested it as well. The point of my question was more about the lack of compiler errors or warnings about doing such an undefined operation.

And indeed, from the link you cited:
Quote
Compilers could warn about such examples, which are typically subtle bugs (or potential subtle bugs). I'm disappointed that after decades, most compilers still don't warn, leaving that job to specialized, separate, and underused tools.



gcc is quite capable of warning about this, but unfortunately the Arduino IDE runs gcc with most warnings disabled.
Formal verification of safety-critical software, software development, and electronic design and prototyping. See http://www.eschertech.com. Please do not ask for unpaid help via PM, use the forum.

Nick Gammon

In verbose mode the sketch above produces:

Code: [Select]

sketch_nov07e.cpp: In function 'void setup()':
sketch_nov07e.cpp:8: warning: operation on 'i' may be undefined
sketch_nov07e.cpp:8: warning: operation on 'i' may be undefined
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

majenko

You know why they don't want you adding your own compiler options?

It's because if you add one of the most common ones to add, and one I always add when crafting a Makefile for a project, -Werror, stops anything at all compiling for the Arduino.

The core library is so full of warnings that it won't compile even with an empty (BareMinimum) sketch:

Here's all the warnings that the core system on 1.01 produces:
Code: [Select]

/home/matt/arduino-1.0.1/hardware/arduino/cores/arduino/HardwareSerial.cpp: In function 'void store_char(unsigned char, ring_buffer*)':
/home/matt/arduino-1.0.1/hardware/arduino/cores/arduino/HardwareSerial.cpp:82: warning: comparison between signed and unsigned integer expressions
/home/matt/arduino-1.0.1/hardware/arduino/cores/arduino/HardwareSerial.cpp: In member function 'virtual size_t HardwareSerial::write(uint8_t)':
/home/matt/arduino-1.0.1/hardware/arduino/cores/arduino/HardwareSerial.cpp:390: warning: comparison between signed and unsigned integer expressions
/home/matt/arduino-1.0.1/hardware/arduino/cores/arduino/Print.cpp: In member function 'size_t Print::print(const __FlashStringHelper*)':
/home/matt/arduino-1.0.1/hardware/arduino/cores/arduino/Print.cpp:44: warning: '__progmem__' attribute ignored
/home/matt/arduino-1.0.1/hardware/arduino/cores/arduino/Tone.cpp:108: warning: only initialized variables can be placed into program memory area


IMHO, no warnings should exist anywhere.  If you get a warning you should get rid of it.

Some of those are unforgivable. 

> "warning: comparison between signed and unsigned integer expressions"

While I agree that these aren't actively causing a problem in the core themselves, they are bad form.  Yes, they may be used for small buffers that are never going to reach 32767 in size, but that is in this current incarnation of the Arduino system.  Create an Arduino with considerably more memory (which is where it is heading with ARM core systems), and the serial buffers could be increased to much bigger.  Hit more than 32767 bytes in the serial buffer without fixing that warning, and bang, you have problems.

And all because someone wrote "int i" instead of "unsigned int i" ...

Just shear laziness.


Go Up