Bug 62184 - [C/C++] Extend -Wempty-body to 'while' loops
Summary: [C/C++] Extend -Wempty-body to 'while' loops
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 5.0
: P3 enhancement
Target Milestone: 6.0
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic, easyhack
Depends on:
Blocks:
 
Reported: 2014-08-19 09:50 UTC by Tobias Burnus
Modified: 2017-06-25 21:29 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-03-03 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Burnus 2014-08-19 09:50:34 UTC

    
Comment 1 Tobias Burnus 2014-08-19 09:52:59 UTC
For an empty "if" loop, GCC warns with -Wextra:

foo.cc:5:13: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
   if (bar());
             ^


However, using a "while" loop, it doesn't. Clang warns (by default) in that case:

foo.cc:5:16: warning: while loop has empty body [-Wempty-body]
  while (bar());
               ^
foo.cc:5:16: note: put the semicolon on a separate line to silence this warning


Again, I found that issue in a real-world code.

Test case:

int bar ();
void sleep();

int foo() {
  while (bar());
    sleep();
  return 1;
}
Comment 2 Manuel López-Ibáñez 2014-08-21 01:13:36 UTC
We had this and it got removed (see PR36478). Perhaps a better implementation is possible now that we track macro locations.

(I wonder how the Clang guys get away with warning about all this stuff by default. Is it because they have very good heuristics to avoid being annoying?)
Comment 3 Tobias Burnus 2014-08-21 07:48:04 UTC
(In reply to Manuel López-Ibáñez from comment #2)
> We had this and it got removed (see PR36478). Perhaps a better
> implementation is possible now that we track macro locations.

There one had:
warn2.cc:6: error: suggest a space before ‘;’ or explicit braces around empty

While CLANG requires a new line (according to the message), such that
#define EMPTY
  while()
    EMPTY;
would be fine while
  while(f()) ; // or: while(f())EMPTY;
wouldn't. (Given how stray spaces enter the code, I'd also like the "while(f()) ;" warning.)


> (I wonder how the Clang guys get away with warning about all this stuff by
> default. Is it because they have very good heuristics to avoid being
> annoying?)

Me too, although I have a log file here with "clang -Weverything" for our code, from which I extract all warning classes (164M) - and look at the individual warnings for those looking potentially interesting. [BTW: One should also consider adding -Weverything to GCC, which helps to find discover warning options.]
Comment 4 Manuel López-Ibáñez 2014-11-12 15:26:49 UTC
I'm going to confirm this. We definitely want this. The heuristic of Clang seems quite elaborated: only warn when the ';' is on the same line and there is a statement after it that starts at a higher column than the while.line.

void foo ()
{
#define EMPTY
  while (0)EMPTY;
    foo(); // warns if preprocessed
  while (0);
   foo(); // warns
  while (0);
  foo(); // does not warn
  while (0); // does not warn
}

$ clang test.c 
test.c:6:12: warning: while loop has empty body [-Wempty-body]
  while (0);
           ^
test.c:6:12: note: put the semicolon on a separate line to silence this warning

$ clang test.c -save-temps
In file included from test.c:1:
test.c:4:12: warning: while loop has empty body [-Wempty-body]
  while (0);
           ^
test.c:4:12: note: put the semicolon on a separate line to silence this warning
test.c:6:12: warning: while loop has empty body [-Wempty-body]
  while (0);
           ^
test.c:6:12: note: put the semicolon on a separate line to silence this warning
Comment 5 Manuel López-Ibáñez 2016-02-15 23:22:48 UTC
If we can warn precisely about misleading indentation, surely we can warn here as well as clang does.
Comment 6 imitrichev 2016-03-02 16:51:31 UTC
I think, while patching one should consider also warn on `for` loop empty body.

Whilst we can write something like

    int i=0;
    for (; i<5; a[i]=10, i++);

it is assumed to be not a great programming style.

    int main()
    {
    	for (; 1<2; );
	if (1==1);
	while (1==1);
	return 0;
    }

Now it warns only on `if` loop:

g++ empty.cpp -Wempty-body -o empty.out

empty.cpp: In function "int main()":
empty.cpp:4:10: warning: suggest braces around empty body in an «if» statement [-Wempty-body]

It is really would be helpful to detect infinite loops caused by odd ';' after `for` and `while` without debugger.
Comment 7 Manuel López-Ibáñez 2016-03-03 00:11:33 UTC
(In reply to imitrichev from comment #6)
> I think, while patching one should consider also warn on `for` loop empty
> body.

Sure, but it is always better to submit a sequence of smaller patches than one large one. Once you get the details right for while() it should be easy to implement for().

> It is really would be helpful to detect infinite loops caused by odd ';'
> after `for` and `while` without debugger.

It just needs someone to implement it. GCC devs are already very busy with other things. We need as much help as we can get.
Comment 8 Patrick Palka 2016-03-25 17:08:15 UTC
Good news, with GCC 6's new -Wmisleading-indentation flag we emit the appropriate warning for the test case in comment #1:

62184.C: In function ‘int foo()’:
62184.C:5:3: warning: this ‘while’ clause does not guard... [-Wmisleading-indentation]
   while (bar());
   ^~~~~
62184.C:6:5: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘while’
     sleep();
     ^~~~~


As for the test case in comment #6:

    int main()
    {
    	for (; 1<2; );
	if (1==1);
	while (1==1);
	return 0;
    }

We don't and arguably shouldn't warn, because none of the statements are indented as if they're intended to be guarded by a previous one.  However, we do warn (twice) for

    int main()
    {
    	for (; 1<2; );
	  if (1==1);
	while (1==1);
	  return 0;
    }

So I think this PR can finally be closed.