Bug 52961 - Make -Wempty-body less noisy and enable it with -Wall
Summary: Make -Wempty-body less noisy and enable it with -Wall
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.8.0
: P3 minor
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: Wall-Wextra
  Show dependency treegraph
 
Reported: 2012-04-12 22:08 UTC by davidxl
Modified: 2024-01-20 22:31 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-08-22 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description davidxl 2012-04-12 22:08:21 UTC
int test(int i) {
  if (i > 42);
    return 42;
  return i;
 }

 /*

 emptyif.cpp:2:14: warning: if statement has empty body [-Wempty-body]
 if (i > 42);
            ^
1 warning generated.

*/
Comment 1 Andrew Pinski 2012-04-12 22:47:23 UTC
I think we had this warning and then removed it.  Let me find the history which I think is in bugzilla already.
Comment 2 Andrew Pinski 2012-04-12 22:49:24 UTC
Yes, see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36478 .
Comment 3 davidxl 2012-04-12 23:11:10 UTC
(In reply to comment #2)
> Yes, see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36478 .

thanks. I tried -Wempty-body, gcc gives warning as expected:


emptyif.cpp: In function 'int test(int)':
emptyif.cpp:2:15: warning: suggest braces around empty body in an 'if' statement [-Wempty-body]
    if (i > 42);
Comment 4 Manuel López-Ibáñez 2012-04-12 23:23:00 UTC
I think that particular warning should be default (not -Wempty-body, but the warning about the if();).

@Jason, Gabriel, do you agree?
Comment 5 Jason Merrill 2012-04-13 19:07:58 UTC
Specifically about if(); without an else, sure.
Comment 6 Paolo Carlini 2012-10-09 16:41:17 UTC
Then, likely this could be resolved very quickly but what in means exactly to have the warning by default here? Change:

	        if (!cp_lexer_next_token_is_keyword (parser->lexer, RID_ELSE))
		  warning_at (loc, OPT_Wempty_body, "suggest braces around "
			      "empty body in an %<if%> statement");

to

	        if (!cp_lexer_next_token_is_keyword (parser->lexer, RID_ELSE))
		  warning_at (loc, 0, "suggest braces around "
			      "empty body in an %<if%> statement");

?? It makes sense to me because I don't think somebody may want to suppress the warning and keep this kind of code, but wanted to confirm.
Comment 7 Manuel López-Ibáñez 2012-10-09 16:58:36 UTC
In any case, there are only 3 warnings in -Wempty-body. All of them seem worth to warn by default.
Comment 8 Manuel López-Ibáñez 2012-10-09 17:06:25 UTC
The problem of default warnings without options is that default warnings are often the most useful and,  hence, the ones that people would like to make an error with -Werror=. Unfortunately, we don't have a -Werror=default to enable all of them.
Comment 9 Paolo Carlini 2012-10-09 17:20:35 UTC
Uhm, I was under the impression that the other 2 used to give problems and that's why we don't warn anymore by default. Bah. All in all, given that we have the warning anyway, I don't think this is really an high priority issue, if you see what I mean ;)
Comment 10 Manuel López-Ibáñez 2014-11-12 15:05:53 UTC
This warning is too noisy because it warns for:

if(a)
  ;
return 2;

which is often the result of macro expansion. Clang specifically does not warn for this and suggests placing the ";" on a different line to silence the warning:


warning: if statement has empty body [-Wempty-body]
  if(a);
       ^
note: put the semicolon on a separate line to silence this warning

which seems a nicer way to silence the warning instead of ugly { ; }
Comment 11 Eric Gallager 2017-08-22 11:26:50 UTC
(In reply to Paolo Carlini from comment #9)
> Uhm, I was under the impression that the other 2 used to give problems and
> that's why we don't warn anymore by default. Bah. All in all, given that we
> have the warning anyway, I don't think this is really an high priority
> issue, if you see what I mean ;)

ok, changing importance from "normal" to "minor" then (and confirming)
Comment 12 Eric Gallager 2019-04-19 06:08:16 UTC
(In reply to Manuel López-Ibáñez from comment #10)
> Clang... suggests placing the ";" on a different line to silence
> the warning:
> 
> 
> warning: if statement has empty body [-Wempty-body]
>   if(a);
>        ^
> note: put the semicolon on a separate line to silence this warning
> 
> which seems a nicer way to silence the warning instead of ugly { ; }

That's a debatable opinion; I think the braces do a better job expressing grouping