Compiler error detection

Development Suggestions for the Arduino Project
1

Hello.

The compiler is far better at identifying errors now than it was in the earlier versions of ide.

However, I inadvertantly added a ';' at the end of an 'if' statement and the compiler did not show an error.

It took me a while to realise what was going on when the code ran on an arduino pro mini.
(It was in a lot of other code).

It effectively skips the comparison part of the if statement and just processes the part in it's associated curly braces.

Can this be addressed for future releases?

Added an example here.... just after Timer[1] there is a semicolon.
this helped me to understand what is actually happening.

Thanks.

[code]

long int Timer[3];
long int LoopNumber;

void setup() {
  // put your setup code here, to run once:

Serial.begin(57600);  
delay (100);

Timer[0] = millis() + 1000;
Timer[1] = Timer[0] + 500;

}

void loop() 
{
  

if (millis() > Timer[0])

{
  Timer[0] = Timer[0] + 1000;
  Serial.print("            Timer[0] = ");Serial.println(Timer[0]);
  Serial.print("LoopNumber = ");Serial.println(LoopNumber);
 
}



if (millis() > Timer[1]);         //  This semicolon should not be here but code compiles ok....

{
    Timer[1] = Timer[1] + 1000;
    Serial.print("Timer[1] = ");Serial.println(Timer[1]);
 Serial.print("LoopNumber = ");Serial.println(LoopNumber);
 
}

LoopNumber++;

}// endof void loop()

[/code]

2

That is perfectly valid C/C++ so the compiler has no way to know that you didn't mean what you wrote.

Pete

3

However, if in File|Preferences you set compiler warnings to "all" you will get this:

C:\Users\Peter\AppData\Local\Temp\arduino_modified_sketch_311944\sketch_sep07b.ino: In function 'void loop()':

C:\Users\Peter\AppData\Local\Temp\arduino_modified_sketch_311944\sketch_sep07b.ino:19:14: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

 if (millis() > Timer[0])

              ^

C:\Users\Peter\AppData\Local\Temp\arduino_modified_sketch_311944\sketch_sep07b.ino:30:14: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

 if (millis() > Timer[1]);         //  This semicolon should not be here but code compiles ok....

              ^

C:\Users\Peter\AppData\Local\Temp\arduino_modified_sketch_311944\sketch_sep07b.ino:30:25: warning: suggest braces around empty body in an 'if' statement [-Wempty-body]

 if (millis() > Timer[1]);         //  This semicolon should not be here but code compiles ok....

                         ^

which apart from warning you about the signed/unsigned comparison, also points out that your if statement has an empty body.
Other compilers, such as MSVC, also give a warning, but not an error, for this type of statement.

Pete

4

Ok. Thanks for that.

So it's an if statement with a null result?

The associated curly braces have no function.

I can't see the point in that so to me it's 'an error'.

Although i sort of understand why ' if (Timer[1] = Timer[0] ) ' doesn't produce an error
but doesn't do what I intended. (needs to be ==).

What's the point in making an assignment in a comparison statement unless it's an unintentional error?

I'll try the 'all' setting for a while, but i suspect at my level of competency, i'll probably not be able to see the wood for the trees :frowning:

Although, I may learn some more useful things.....

Thanks again.

5

So it's an if statement with a null result?

Yes, unless there is a side-effect hidden in the condition such as:

 if((inchar = Serial.read()) == 'a');

The test doesn't do anything but the side-effect is that inchar is set to the character read from serial input.

The associated curly braces have no function.

Yup. They just group the statements within them but when they aren't associated correctly with the if statement it doesn't really matter what happens with them.

I agree that in this case an if statement with nothing following it, even if it has side-effects, is really an error but the compiler writers have at least warned you of the problem.
It is actually a very good idea to have all warnings enabled and hunt them down just as diligently as you would any errors.
Here's a couple of examples why:

warning: array subscript is above array bounds [-Warray-bounds]

This one is fairly obvious. If you have an array, foo, declared to have 10 elements and you try to access foo[10] you'll run into problems which can be really hard to find.

In this code:

 if (IsDead && DomeOpen)

the error message was:

warning: the address of 'boolean IsDead()' will always evaluate as 'true'

Even out of context, the warning actually tells you precisely what is wrong. IsDead is a function (not a variable) which is why it refers to 'boolean IsDead()' and what is required is that you use isDead() in the if statement so that the function is called. Even reasonably experienced programmers would have fun debugging this problem if they ignored warnings and this statement was buried in among a thousand other lines of code.

I try, especially after the first few compilations of new code, to make a habit of going through all the warnings to make sure that none of them are actually bugs in disguise like the two examples above.

Pete

6

JohnSan:
What's the point in making an assignment in a comparison statement unless it's an unintentional error?

char c;
while(someStreamObject.available()){
   if(c = someStreamObject.read()){
          // c is a good character, do something with it
   }
   else {
          // c is a null character.  Maybe that marks the end of a packet or something
   }
}
7

JohnSan:
I can't see the point in that so to me it's 'an error'.

But the definition of an error is something the compiler can't understand, not the programmer. It's a good thing too. IF the compiler puked on every single line that the programmer didn't understand then the noobies would never stand a chance. Every single line would get marked error and someone else would have to compile their code for them.