27

I'm using Keil µVision v4.74 and have enabled the option "All Warnings".

I wrote the following intentional code:

if(condition matched)
{
  // Do something
}

When I rebuilt my project, I got 0 errors, 0 warnings.

However, when I accidentally wrote:

if(condition matched);
{
  // Do something
}

I also got 0 errors, 0 warnings.

It was next to impossible for me to find out that a small ; following the if condition was the root of the problem.

Why didn't the compiler treat it as a warning and inform me?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
AlphaGoku
  • 968
  • 1
  • 9
  • 24
  • 11
    Why should he? Thats a perfectly well formed and legal statement. Nothing warnable here. – ckruczek May 27 '16 at 11:59
  • 13
    When i declare a variable and dont use it, he say "variable declared but undefined". Should he say something like "if statement used but no body for if statement?" – AlphaGoku May 27 '16 at 12:00
  • 2
    One thing I want to mention here. This is not at all related to `keil`.. in general programming also it will happen. – someone May 27 '16 at 12:17
  • i tagged it to keil because i expected someone from keil may be able to tell why or consider this to improve their IDE. I dont want someone using Keil to go through the same problem i went through. gcc throws a warning as told by @Matteo – AlphaGoku May 27 '16 at 12:19
  • 4
    The compiler should output one and only one warning, “You are using c, don’t expect a safety net”. – Ian Ringrose May 27 '16 at 14:08
  • @Vector9 I agree, I can't think of any legitimate reason for declaring an `if` statement without a body. Seems like it should warn by default, without requiring you to ask the compiler for extra warnings. – Dmiters May 27 '16 at 17:04
  • 3
    @IanRingrose: on the contrary, given that the runtime is completely without safety nets for performance reasons, having all the possible static analysis at compile time is extremely desirable. – Matteo Italia May 27 '16 at 18:20
  • @MatteoItalia, then don't user C or C++! Module 2 would be a lot better option, if you care about safety. – Ian Ringrose May 27 '16 at 18:22
  • 6
    @ckruczek But... that's exactly what warnings are for. If it weren't well formed and legal, it would be an error. Warnings are for things that are legal, but suspicious as likely logic errors. – GrandOpener May 27 '16 at 18:58
  • 2
    Keep in mind that many macro suites create this sort of situation right and left, as bits of code are "commented out" or not based on global macro variable settings. – Hot Licks May 27 '16 at 20:23
  • @HotLicks That sounds interesting to me because I can't think of a case where it wouldn't make just as much to either 1) "comment out" the `if` block as well, or 2) invert the conditional and make the statement that gets "commented out" an `else` block or something similar. So that situation seems entirely avoidable, but being that I'm not a C expert, maybe there's just things I'm not thinking of. – jpmc26 May 27 '16 at 21:17
  • 1
    @jpmc26 - Consider if I do `if (a) some_dump_macro;`. If a global flag somewhere turns off `some_dump_macro` then I end up with `if (a);`. – Hot Licks May 27 '16 at 21:49
  • because keil compiler is not very user-friendly – M.M May 28 '16 at 12:55

2 Answers2

51

It's not an error because an empty statement is a valid statement; however, since it's certainly suspicious code it's the perfect candidate for a compiler warning - and in fact gcc -Wall -Wextra does warn about this:

int foo(int x) {
  if(x); {
    return 42;
  }
  return 64;
}

 

/tmp/gcc-explorer-compiler116427-37-l1vpg4/example.cpp: In function 'int foo(int)':
2 : warning: suggest braces around empty body in an 'if' statement [-Wempty-body]
if(x); {
^

https://godbolt.org/g/RG1o7t

Both Clang and Visual C++ do it too.

GCC 6 is even smarter (well, maybe too much), and takes even the indentation as a hint that something is wrong:

/tmp/gcc-explorer-compiler116427-76-1sfy0y/example.cpp: In function 'int foo(int)':
2 : warning: suggest braces around empty body in an 'if' statement [-Wempty-body]
if(x); {
^
2 : warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if(x); {
^~
2 : note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if'
if(x); {
^

So, either you don't have the warnings cranked up enough, or your compiler isn't smart enough.

If you don't have the possibility to switch to a more helpful compiler, consider using static analysis tools; for example, in this case cppcheck spots the error (when given the --enable=all --inconclusive flags):

cppcheck --enable=all --inconclusive emptyif.c

Output:

Checking emptyif.c...
[emptyif.c:2]: (warning, inconclusive) Suspicious use of ; at the end of 'if' statement.
[emptyif.c:1]: (style) The function 'foo' is never used.

Addendum - relevant warnings for various compilers (feel free to update)

To recap, the relevant warning options are:

  • gcc -Wempty-body; included in -Wextra;
  • gcc>=6.0, also -Wmisleading-indentation can help; included in -Wall;
  • Clang -Wempty-body; included in -Wextra too;
  • Visual C++ C4390, included in /W3

Static analysis tools:

  • cppcheck --enable=warning --inconclusive; included in --enable=all --inconclusive
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Matteo Italia
  • 123,740
  • 17
  • 206
  • 299
  • How much more can you crank Keil up beyond "All Warnings"? – AlphaGoku May 27 '16 at 12:12
  • @Vector9: I don't know, I never used Keil. Besides, it seems like Keil itself is mostly an IDE, what compiler backend does it use? Unfortunately "minor" compilers for embedded platforms often lag way behind "mainstream" compilers in this kind of diagnostics. – Matteo Italia May 27 '16 at 12:13
  • My keil uses Armcc.exe v5.03.0.76 – AlphaGoku May 27 '16 at 12:22
  • 3
    @Vector9: giving a quick look to the list of warnings supported by armcc there doesn't seem to be anything related to this issue. However, I see that Keil [does support](http://www.keil.com/arm/gnu.asp) using `gcc`, so you may give it a try (maybe not in production, but just from time to time to have a look at a different set of compiler diagnostics). But again, I'm no Keil/armgcc expert, so I don't know if can be used mostly as a drop-in replacement or if it is going to be a nightmare to switch between compilers. – Matteo Italia May 27 '16 at 12:28
  • 4
    Yup, MSVC warns: `warning C4390: ';': empty controlled statement found; is this the intent?`. It's actually a [level 3 warning](https://msdn.microsoft.com/en-us/library/8f1zx4y1.aspx), so you don't even need /W4. The default warning level will catch this one. – Cody Gray - on strike May 27 '16 at 14:34
  • Is there an analogue flag for `clang` that would produce this warning? – mucaho May 27 '16 at 15:30
  • 1
    @mucaho: `-Wextra` includes it even in clang; the specific flag for this warning is the same as for `gcc`, i.e. `-Wempty-body`. – Matteo Italia May 27 '16 at 15:38
  • Lint will warn about the unexpected change in indentation as well. – davenpcj May 27 '16 at 18:04
  • 1
    @davenpcj Suggesting the use of a linter may be worth posting as an answer, since it (presumably) wouldn't require switching compilers. – jpmc26 May 27 '16 at 21:10
3

As Matteo's answer indicated, the code is absolutely valid. It's being interpreted this way:

if(condition)
    ;  // do nothing

// unrelated block
{
    // do something
}

It's a bit of a technicality, but conditions with empty bodies do have some very nice uses.

Lint and other such code sanity tools will warn about the unexpected change in indentation, and catch additional errors that may be stylistic though not technically compiler errors.

Or security problems, variable tainting, buffer management, potential maintenance problems like bad casts, etc. There are an awful lot of code problems that don't fall into the category of "compiler errors".

As jpmc26 mentioned, this approach may be better since you don't have to switch compilers to use it. Though I also personally find value in the ability to run the two independently.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
davenpcj
  • 12,508
  • 5
  • 40
  • 37