Bug 36478

Summary: [4.3 regression] warning not emitted when code expanded from macro
Product: gcc Reporter: Tom Tromey <tromey>
Component: c++Assignee: Jakub Jelinek <jakub>
Status: RESOLVED FIXED    
Severity: normal CC: dodji, fang, fedora84, gcc-bugs
Priority: P2 Keywords: diagnostic
Version: 4.3.0   
Target Milestone: 4.3.6   
URL: http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00361.html
See Also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52961
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2008-11-10 13:53:26
Attachments: candidate partial fix

Description Tom Tromey 2008-06-09 16:33:18 UTC
This is a copy of a bug report from Red Hat's bugzilla:
https://bugzilla.redhat.com/show_bug.cgi?id=449191

The bug is that with mapped locations, the code in check_empty_body
does not notice that the while() from the macro expansion violates the
rule.  This is because of the hack we needed to get decent error messages
with mapped locations -- tokens resulting from a macro expansion are all
given the location of the start of the expansion.


Steps to Reproduce:
1. compile example program:

> cat warn2.cc
int
main(int, char**)
{
#define XXX while(0);
    XXX ;
    while (0);
}

> g++ -W -Werror -c warn2.cc
  
Actual results:
cc1plus: warnings being treated as errors
warn2.cc: In function ‘int main(int, char**)’:
warn2.cc:6: error: suggest a space before ‘;’ or explicit braces around empty
body in ‘while’ statement


Expected results:
warn2.cc: In function ‘int main(int, char**)’:
warn2.cc:5: warning: suggest a space before ‘;’ or explicit braces around empty
body in ‘while’ statement
warn2.cc:6: warning: suggest a space before ‘;’ or explicit braces around empty
body in ‘while’ statement
Comment 1 Richard Biener 2008-06-10 09:50:40 UTC
Confirmed.
Comment 2 Dodji Seketeli 2008-07-31 12:22:35 UTC
Created attachment 15984 [details]
candidate partial fix

This patch fixes the problem patially.

if you have:
------------
void  
foo ()
{
#define WARNS while(0)
#define DOES_NOT_WARN while(0); 
        WARNS; // { dg-warning }
        DOES_NOT_WARN;
            while (0); // { dg-warning }
}
---------------

The compiler (augmented with the patch) says:
/home/dodji/devel/src/gcc-bug-test.cc:6: warning: suggest a space before ‘;’ or explicit braces around empty body in ‘while’ statement
/home/dodji/devel/src/gcc-bug-test.cc:8: warning: suggest a space before ‘;’ or explicit braces around empty body in ‘while’ statement
Comment 3 Manuel López-Ibáñez 2008-08-11 12:32:16 UTC
(In reply to comment #2)
> void  
> foo ()
> {
> #define WARNS while(0)
> #define DOES_NOT_WARN while(0); 
>         WARNS; // { dg-warning }
>         DOES_NOT_WARN;

I don't think we want to warn here. Even if we have the right location of ')', this would be hard to warn. How do you distinguish?

DOES_NOT_WARN;
DOES_NOT_WARN/**/;

That said, I don't think tracking the end of the macro is a good idea either. It just moves the problem around. Another workaround would be to not check close_paren.column + 1 == semicolon.column since it is the next token and before the semicolon there is no whitespace. The only problem is if there is a comment, such as while(0)/**/; and we have no way to track that, have we?.

For a proper fix, we need at least 2 locations for each token that comes from an expansion. The place where the token really appeared and the place where it ended up. However, this is not trivial. We need either to encode such information in 1 location_t number somehow or to encode it in the token.
Comment 4 Joseph S. Myers 2008-08-27 22:04:34 UTC
4.3.2 is released, changing milestones to 4.3.3.
Comment 5 Jakub Jelinek 2008-11-10 10:15:36 UTC
2 locations won't help you either.
void foo ()
{
#define EMPTY
  while (0)EMPTY;
}
won't warn when compiled with g++ -Wempty-body -O2, but will with
g++ -Wempty-body -O2 -save-temps (i.e. ccache etc.).  I think it is unfortunate
this -Wempty-body extension was accepted for 4.3, and if we want to make it work, we really have to handle it at the preprocessor level.
Comment 6 Jakub Jelinek 2008-11-10 13:53:26 UTC
Patch posted.
Comment 7 Jakub Jelinek 2008-11-12 22:19:33 UTC
Subject: Bug 36478

Author: jakub
Date: Wed Nov 12 22:18:03 2008
New Revision: 141810

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141810
Log:
	PR c++/36478
	Revert:
	2007-05-07  Mike Stump  <mrs@apple.com>
	* doc/invoke.texi (Warning Options): Document that -Wempty-body
	also checks for and while statements in C++.

	Revert:
	2007-05-07  Mike Stump  <mrs@apple.com>
	* parser.c (check_empty_body): Add.
	(cp_parser_iteration_statement): Add call to check_empty_body.

	* g++.old-deja/g++.mike/empty.C: Remove.

Removed:
    trunk/gcc/testsuite/g++.old-deja/g++.mike/empty.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/parser.c
    trunk/gcc/doc/invoke.texi
    trunk/gcc/testsuite/ChangeLog

Comment 8 Richard Biener 2009-01-24 10:20:28 UTC
GCC 4.3.3 is being released, adjusting target milestone.
Comment 9 Richard Biener 2009-08-04 12:29:14 UTC
GCC 4.3.4 is being released, adjusting target milestone.
Comment 10 Richard Biener 2010-05-22 18:12:28 UTC
GCC 4.3.5 is being released, adjusting target milestone.
Comment 11 Dodji Seketeli 2010-11-26 15:29:59 UTC
Fixed in 4.4+ only as the -Wempty-body patch was reverted.

The underlying problem can be tracked by PR7263.