name lookup of 'i' changed for new ISO 'for' scoping

Oh, and I often also use -Werror, so I can't compile with any warnings :wink:

How often does this happen?

for(ptr=head; ptr->next; ptr++);
{
  printf("Hello!");
}

It makes sense to warn about that. If the programmer intended for it to be for->semicolon->blockofcode, they could do this:

for(ptr=head; ptr->next; ptr++) {}
{
  printf("Hello!");
}

However, obviously no warning should be shown for this:

for(ptr=head; ptr->next; ptr++);
printf("Hello!");

or this

for(ptr=head; ptr->next; ptr++) printf("Hello!");

and only maybe this

for(ptr=head; ptr->next; ptr++) printf("hi");
{
  printf("Hello!");
}

A warning for a code block with no parent if/for/while etc is valid, yes. You can't give warnings for bad indentation, unless you want to program in COBOL.

majenko:
A warning for a code block with no parent if/for/while etc is valid, yes.

What? No way. There are plenty of valid reasons to use a block with no statement, ranging from controling when things are destroyed (especially important for the ATOMIC_BLOCK things) to being able to declare new variables in a switch statement.

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).

dc42:
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

Nice cartoon!

Nantonos:

[quote author=Nick Gammon link=topic=130491.msg981373#msg981373 date=1351937902]
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).
[/quote]

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.

Nantonos:
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.

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++

self.foot(shoot)

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

eg.

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:

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:

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:

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:

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. :wink:

[quote author=Nick Gammon link=topic=130491.msg984794#msg984794 date=1352181831]
This is good in a way, but now you can't do this:

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

matching_index = i;

Because "i" is now out of scope. [/quote]

True, but you can do this

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.

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

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

void loop()
{
}

The compiler has no problem with

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

is the result 3, or 4?

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: Sequence point - Wikipedia

The value of i++ between sequence points is undefined (that is, it might, or might not, be incremented yet).

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:

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.

Nantonos:

[quote author=Nick Gammon link=topic=130491.msg986067#msg986067 date=1352255745]
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:

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.

[/quote]

gcc is quite capable of warning about this, but unfortunately the Arduino IDE runs gcc with most warnings disabled.

In verbose mode the sketch above produces:

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

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:

/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.