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

majenko:
So how would you suggest suppressing a warning for

for(ptr=head; ptr->next; ptr++);

?

By the way, the precompiler strips all comments, so putting comments in something is a futile exercise.

Only if the preprocessor (or precompiler as you call it) is a separate program from the main compiler, which is rarely the case these days.

majenko:
An empty block gets reduced to an end of statement (;).

It may be treated in the same way as end-of-statement in some contexts, but the compiler knows the difference, so it is easy for the compiler writter to generate a warning in one case but not in the other.

majenko:
You can't have a warning for something that is valid, just because in that one specific situation the programmer used it wrong.

Rubbish. There are very many things that are valid in C++ but symptomatic of possible or likely errors, and all good C++ compilers can warn about many of them. You can choose to run the compiler at its minimum warning level and see none of these warnings, or the maximum warning level and see all of them, or somewhere in between. See Warning Options (Using the GNU Compiler Collection (GCC)) for the warning options supported by the gcc compiler used by the Arduino IDE. In particular, note the following:

-Wall
This enables all the warnings about constructions that some users consider questionable, and that are easy to avoid (or modify to prevent the warning), even in conjunction with macros.

An if, while, for, etc does not "own" the { and }. It merely expects a valid statement.

; is a valid statement.

moveServoToPosition(90); is a valid statement.

{ moveServoToPosition(90); } is a valid statement.

{ } is a valid statement.

The { and } merely convert the containing statements into a single compound statement as seen by the parent layer (the if, while, whatever).

While I agree that an if statement with no body (if(x==3):wink: is syntactically correct, it is pointless, and yes a warning "If without body" could be generated. However, we're not arguing about ifs here - if was introduced about half way through the discussion by WizenedEE.

The code

for(ptr=head; ptr->next; ptr++);

and the code

for (int i = 1500; i >= 1000; i --);

are both perfectly syntactically correct, and in some circumstances are programatically correct as well - i.e., they are exactly what the programmer intended.

There is no way for a compiler to know what a programmer intends.

If you want to start forcing a { } to indicate an intentional empty body, then two things are happening:

  1. You are forcing a coding style on someone. This is not something that C does. Python imposes coding styles. COBOL imposes coding styles. Upper management impose coding styles. C doesn't, it never has, and it never will.

  2. It will break the C specification, which states that the above code is valid.

And then, will you warn about:

for(x=0; x<10; x++)
  doSomething(x);

just because the doSomething(x) isn't inside a { and } ?

This enables all the warnings about constructions that some users consider questionable, and that are easy to avoid (or modify to prevent the warning), even in conjunction with macros.

None of the above three chunks of code are in any way questionable, vague, or otherwise suspect. They are perfectly valid.

Modifying them to "prevent the warning" involves things like adding extra brackets around assignments, etc - not changing the whole structure of the C language!

majenko:
The code

for(ptr=head; ptr->next; ptr++);

and the code

for (int i = 1500; i >= 1000; i --);

are both perfectly syntactically correct, and in some circumstances are programatically correct as well - i.e., they are exactly what the programmer intended.

Whilst I agree that those examples are not quite in the same category as "if (x = 8)", I think the fact that the OP made the mistake of putting in that semicolon and had to ask for help to find it demonstrates the usefulness of a compiler warning for this construct.

majenko:
If you want to start forcing a { } to indicate an intentional empty body, then two things are happening:

  1. You are forcing a coding style on someone. This is not something that C does. Python imposes coding styles. COBOL imposes coding styles. Upper management impose coding styles. C doesn't, it never has, and it never will.

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. But Arduino is intended for hobbyists and novices, so the situation here is less clear cut. I would like the Arduino IDE to provide a choice of warning levels, or some way of passing additional flags to gcc.

majenko:
2. It will break the C specification, which states that the above code is valid.

Warning about questionable code does not break the C specification. Refusing to compile it would.

majenko:
None of the above three chunks of code are in any way questionable, vague, or otherwise suspect. They are perfectly valid.

We'll have to agree to differ about that.

majenko:
Modifying them to "prevent the warning" involves things like adding extra brackets around assignments, etc - not changing the whole structure of the C language!

Requiring the programmer to put the semicolon on a different line from the end of the loop header (in order to make his intention clear and suppress the warning) does not change the structure of the C language either. [Although all C coding standards for safety-critical software I have seen do make { } compulsory in the bodies of all loops and if-statements, in order to reduce the incidence of errors.]

BTW I believe gcc does warn about for-loops of this form if you use the -Wextra or -Wempty-body compiler options. It's not documented on the page I linked to, but I found a note that gcc extends that warning to include for-loops from gcc 4.3.

Whilst I agree that those examples are not quite in the same category as "if (x = 8)", I think the fact that the OP made the mistake of putting in that semicolon and had to ask for help to find it demonstrates the usefulness of a compiler warning for this construct.

No, it highlights the fact that, given the target market for the Arduino, the C language was a very poor choice. It is a very complex language, and easy to make mistakes if you don't know what you're doing - and in the land of the Arduino, 9 times out of 10, the "programmers" don't know what they are doing. This is often the first time they have written any code in their lives.

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. But Arduino is intended for hobbyists and novices, so the situation here is less clear cut. I would like the Arduino IDE to provide a choice of warning levels, or some way of passing additional flags to gcc.

The coding style starts with the user. If they don't know what they're doing, no amount of forced coding styles (which I abhor - coding styles are personal), and warnings (which on the Arduino are pointless, because a) the line numbers are wrong, and b) people don't read them, they just post snippets of them on here and ask what is wrong), will make them better programmers.

I could sit here all day and go on about things that could be improved with the Arduino so called "IDE", and yes, the ability to set compiler options (warnings, and such) is pretty high on my list. But it does come below having an IDE that is actually usable for anything more than writing short little test snippets of code.

majenko:
The coding style starts with the user. If they don't know what they're doing, no amount of forced coding styles ... and warnings (which on the Arduino are pointless, because a) the line numbers are wrong, and b) people don't read them, they just post snippets of them on here and ask what is wrong), will make them better programmers.

But extra warnings help people who mostly do know what they are doing avoid making silly mistakes. I once had to convert 4 million lines of C++ code, written by programmers who were extremely competent, to be compatible with a new compiler and its associated libraries. At the same time, I increased the compiler warning level to near maximum. The new warnings revealed more than 200 bugs in the code. This was in code that had been tested and released.

If the line number in Arduino messages are wrong sometimes, that should be fixed. I find the error message line numbers are mostly OK.

majenko:
(which I abhor - coding styles are personal)

I guess you are either a hobbyist or a one-developer company, because that attitude is not workable in a multi-developer team. In safety-critical work, companies mostly pick one of a few standard coding rule sets such as MISRA-C, MISRA-C++ and JSF C++.

I've not commented on your other points because I agree with them at least in part.

dc42:

majenko:
The coding style starts with the user. If they don't know what they're doing, no amount of forced coding styles ... and warnings (which on the Arduino are pointless, because a) the line numbers are wrong, and b) people don't read them, they just post snippets of them on here and ask what is wrong), will make them better programmers.

But extra warnings help people who mostly do know what they are doing avoid making silly mistakes. I once had to convert 4 million lines of C++ code, written by programmers who were extremely competent, to be compatible with a new compiler and its associated libraries. At the same time, I increased the compiler warning level to near maximum. The new warnings revealed more than 200 bugs in the code. This was in code that had been tested and released.

That I agree with. I always code with -Wall turned on.

If the line number in Arduino messages are wrong sometimes, that should be fixed. I find the error message line numbers are mostly OK.

majenko:
(which I abhor - coding styles are personal)

I guess you are either a hobbyist or a one-developer company, because that attitude is not workable in a multi-developer team. In safety-critical work, companies mostly pick one of a few standard coding rule sets such as MISRA-C, MISRA-C++ and JSF C++.

One developer company at the moment, but I have worked in large development teams. I find my personal coding style fits in pretty well with others - and I am willing to change my coding style to fit in with management's desires - but at the end of the day I believe that the coding style that is most comfortable for an experienced programmer to work with is the one least prone to them making mistakes in.

"Standard" coding styles are only really there so that other developers in the team can understand easily what you have written. That and comments (something else I am not good at :wink: ).

What gets most confusing though is when you are part of a project team with multiple programming languages involved, and you have different programming styles imposed upon you for each language - either by management (which I find is often the case of you conforming to your boss's style of programming, and not to any particular "standard" - at least in the industries I have worked in), or the differences in language structures.

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?