#define A(a) #define A(b) g++-4.3 -S t.C t.C:2:1: error: "A" redefined t.C:1:1: error: this is the location of the previous definition We check the parameter names before the expansion, but if the expansion is empty in both cases there is no need to warn about different names. Of course watch out to still warn for #define A(a) a #define A(b) a which is why this only works for empty macros. The following would work for example: Index: macro.c =================================================================== --- macro.c (revision 129646) +++ macro.c (working copy) @@ -1281,6 +1281,12 @@ warn_of_redefinition (cpp_reader *pfile, || macro1->variadic != macro2->variadic) return true; + /* If the macro expansion has no tokens there is no need to compare + parameters spellings. */ + if (macro1->count == 0 + && macro2->count == 0) + return false; + /* Check parameter spellings. */ for (i = 0; i < macro1->paramc; i++) if (macro1->params[i] != macro2->params[i])
The standard requires the spelling of parameters to be the same even if the replacement list is empty.
I know - I should have made this an enhancement request. Certainly there will be no observable difference for empty replacement lists, no?
The difference is that it is a constraint. You must diagnose it at least in pedantic mode.
Subject: Re: New: Empty macro definitions not considered equal On Fri, 26 Oct 2007, rguenth at gcc dot gnu dot org wrote: > #define A(a) > #define A(b) > > g++-4.3 -S t.C > t.C:2:1: error: "A" redefined > t.C:1:1: error: this is the location of the previous definition > > We check the parameter names before the expansion, but if the expansion is > empty in both cases there is no need to warn about different names. Both C and C++ require diagnostics here, my interpretation being that they are required even if the only spelling difference is a different UCN for the same identifier (bug 9449 comment 39).
Given that this is a constraint, my first inclination is to close the bug report. Richard, what motivated this PR?
Subject: Re: Empty macro definitions not considered equal On Sun, 25 Nov 2007, tromey at gcc dot gnu dot org wrote: > ------- Comment #5 from tromey at gcc dot gnu dot org 2007-11-25 21:11 ------- > Given that this is a constraint, my first inclination is to close the bug > report. > > Richard, what motivated this PR? Complaints that we now diagnose those as error for C++ (where -pedantic-errors is default and that now infects the preprocessor behavior, which is what changed compared to 4.2). But yes, there's probably nothing else than to close this bug. Richard.
(In reply to comment #6) > > But yes, there's probably nothing else than to close this bug. > Well you could make the error depend on the pedantic flag. This is a recurrent confusion: C++ does not enable pedantic-errors by default. It makes pedwarns as errors. -pedantic-errors is pedantic flag + pedwarns as errors. If you can provide me with a better text for the comment (something about the part of the standard that requires us to give a diagnostic), I can update the patch, bootstrap it and regression test it. Index: libcpp/macro.c =================================================================== --- libcpp/macro.c (revision 130380) +++ libcpp/macro.c (working copy) @@ -1284,6 +1284,12 @@ || macro1->variadic != macro2->variadic) return true; + + /* If the macro expansion has no tokens there is no need to compare + parameters spellings unless -pedantic was given. */ + if (!CPP_PEDANTIC (pfile) && macro1->count == 0 && macro2->count == 0) + return false; + /* Check parameter spellings. */ for (i = 0; i < macro1->paramc; i++) if (macro1->params[i] != macro2->params[i]) Index: gcc/testsuite/g++.dg/cpp/redefine-empty-macro-pedantic.C =================================================================== --- gcc/testsuite/g++.dg/cpp/redefine-empty-macro-pedantic.C (revision 0) +++ gcc/testsuite/g++.dg/cpp/redefine-empty-macro-pedantic.C (revision 0) @@ -0,0 +1,4 @@ +// { dg-do preprocess } +// { dg-options "-pedantic" } +#define A(a) // { dg-error "this is the location of the previous definition" } +#define A(b) // { dg-error ".A. redefined" } Index: gcc/testsuite/g++.dg/cpp/redefine-empty-macro.C =================================================================== --- gcc/testsuite/g++.dg/cpp/redefine-empty-macro.C (revision 0) +++ gcc/testsuite/g++.dg/cpp/redefine-empty-macro.C (revision 0) @@ -0,0 +1,4 @@ +// { dg-do preprocess } +// { dg-options "" } +#define A(a) +#define A(b)
Subject: Re: Empty macro definitions not considered equal On Sun, 25 Nov 2007, manu at gcc dot gnu dot org wrote: > ------- Comment #7 from manu at gcc dot gnu dot org 2007-11-25 22:23 ------- > (In reply to comment #6) > > > > But yes, there's probably nothing else than to close this bug. > > > > Well you could make the error depend on the pedantic flag. This is a recurrent > confusion: C++ does not enable pedantic-errors by default. It makes pedwarns as > errors. -pedantic-errors is pedantic flag + pedwarns as errors. But C++ has -pedantic as default as well ;) > If you can provide me with a better text for the comment (something about the > part of the standard that requires us to give a diagnostic), I can update the > patch, bootstrap it and regression test it. Tom can probably do this. But I belive the patch will not work, as CPP_PEDANTIC is set to true by the C++ frontend now. Richard.
(In reply to comment #8) > > But C++ has -pedantic as default as well ;) > How you reached to that conclusion? > > Tom can probably do this. But I belive the patch will not work, > as CPP_PEDANTIC is set to true by the C++ frontend now. > Again, how you reached to that conclusion and what you mean by now?
(In reply to comment #8) > > Tom can probably do this. But I belive the patch will not work, > as CPP_PEDANTIC is set to true by the C++ frontend now. BTW, the patch works for the revision I diffed against. The testcases included in the patch pass.
Subject: Re: Empty macro definitions not considered equal On Sun, 25 Nov 2007, manu at gcc dot gnu dot org wrote: > ------- Comment #10 from manu at gcc dot gnu dot org 2007-11-25 22:59 ------- > (In reply to comment #8) > > > > Tom can probably do this. But I belive the patch will not work, > > as CPP_PEDANTIC is set to true by the C++ frontend now. > > BTW, the patch works for the revision I diffed against. The testcases included > in the patch pass. I see. But sth changed in the cpp defaults for C++ in 4.3 as things that were previously warnings (with 4.2) are now errors (with 4.3), such as this one or the macro re-definition. Richard.
Subject: Re: Empty macro definitions not considered equal On Mon, 26 Nov 2007, rguenther at suse dot de wrote: > I see. But sth changed in the cpp defaults for C++ in 4.3 as things > that were previously warnings (with 4.2) are now errors (with 4.3), such > as this one or the macro re-definition. That would have been the fix for bug 24924.
(In reply to comment #11) > I see. But sth changed in the cpp defaults for C++ in 4.3 as things > that were previously warnings (with 4.2) are now errors (with 4.3), such > as this one or the macro re-definition. > (My english must be getting pretty bad lately, please let me know what part is not clear. Perhaps we should put up some Developers FAQ in the wiki.) * C++ front-end did not and does not enable -pedantic-errors by default. * C++ front-end makes pedwarns to be emitted as errors. * -pedantic-errors sets the variable pedantic to true and makes pedwarns to be emitted as errors. * C++ front-end in 4.3 now makes CPP's pedwarns to be emitted as errors. * C++ front-end in 4.3 did not and does not set CPP's pedantic flag to true, you still need -pedantic or -pedantic-errors for that. * So, a CPP's pedwarn that is guarded by the pedantic flag still requires an explicit -pedantic or -pedantic-errors in the command-line to be generated (as error by default, as a warning with -fpermissive). Please, I am banging my head against the wall since comment #7 of why this is not clear. So, please, let me know which part I am explaining wrong.
Subject: Re: Empty macro definitions not considered equal On Mon, 26 Nov 2007, manu at gcc dot gnu dot org wrote: > ------- Comment #13 from manu at gcc dot gnu dot org 2007-11-26 13:03 ------- > (In reply to comment #11) > > I see. But sth changed in the cpp defaults for C++ in 4.3 as things > > that were previously warnings (with 4.2) are now errors (with 4.3), such > > as this one or the macro re-definition. > > > > (My english must be getting pretty bad lately, please let me know what part is > not clear. Perhaps we should put up some Developers FAQ in the wiki.) > > * C++ front-end did not and does not enable -pedantic-errors by default. > > * C++ front-end makes pedwarns to be emitted as errors. > > * -pedantic-errors sets the variable pedantic to true and makes pedwarns to be > emitted as errors. > > * C++ front-end in 4.3 now makes CPP's pedwarns to be emitted as errors. > > * C++ front-end in 4.3 did not and does not set CPP's pedantic flag to true, > you still need -pedantic or -pedantic-errors for that. > > * So, a CPP's pedwarn that is guarded by the pedantic flag still requires an > explicit -pedantic or -pedantic-errors in the command-line to be generated (as > error by default, as a warning with -fpermissive). > > Please, I am banging my head against the wall since comment #7 of why this is > not clear. So, please, let me know which part I am explaining wrong. I guess you are not explaining it wrong, but the situation is extremely confusing: /* Adjust various flags for C++ based on command-line settings. */ if (c_dialect_cxx ()) { if (!flag_permissive) { flag_pedantic_errors = 1; cpp_opts->pedantic_errors = 1; } ... } which means by default pedantic is false but pedantic-errors is true (though -fpedantic-errors also sets pedantic to true). So all pedwarns() that are not guarded by an extra if (pedantic) are errors for C++ (and for libcpp now). So, to get regular -pedantic behavior you need -fpermissive -pedantic (?) (because obviously the C++ default is not what you get from enabling any of the -pedantic -pedantic-errors or -fpermissive flags) And of course there's no -no-pedantic-errors either. I find this situation confusing (especially with the defintion of pedwarn, which reads /* A "pedantic" warning: issues a warning unless -pedantic-errors was given on the command line, in which case it issues an error. Use this for diagnostics required by the relevant language standard, if you have chosen not to make them errors. Note that these diagnostics are issued independent of the setting of the -pedantic command-line switch. To get a warning enabled only with that switch, write "if (pedantic) pedwarn (...);" */ void pedwarn (const char *gmsgid, ...) so it suggests -pedantic-errors, but in reality it only checks for flag_pedantic_errors: diagnostic.h:#define pedantic_error_kind() (flag_pedantic_errors ? DK_ERROR : DK_WARNING) So, for C++ you either get no warning by default for if (pedantic) pedwarn (...); or you get an error by default for pedwarn (...); But you cannot get a warning for pedwarn as its name suggests. (-fpermissive -pedantic should work, but maybe doesn't) Richard.
(In reply to comment #14) > > I guess you are not explaining it wrong, but the situation is > extremely confusing: > Oh, I fully agree on that. But it is exactly the same situation that was there pre-4.3 for the C++ front-end. It is just that now CPP and C++ front-end defaults are consistent. > So, to get regular -pedantic behavior > you need -fpermissive -pedantic (?) (because obviously the C++ default > is not what you get from enabling any of the -pedantic -pedantic-errors > or -fpermissive flags) And of course there's no -no-pedantic-errors > either. That is a good point. But it is not new. It seems to have been like this for the C++ front-end forever. > But you cannot get a warning for pedwarn as its name suggests. > (-fpermissive -pedantic should work, but maybe doesn't) -fpermissive gets a warning for pedwarn(). -fpermissive -pedantic gets you a warning for "if (pedantic) pedwarn()". Think about it like this: pedwarns are diagnostics required by the standard. Some of them are nice, others are a pain in the ass. So we "hide" away the latter under "if (pedantic)" or more precisely under "-pedantic". Now, why the C++ front-end has pedwarns as errors by default, I don't know. But if you have that, then you need -fpermissive. In the C front-end there is no flag to get pedwarns as warnings back: once you issue -pedantic-errors in the command-line, then you cannot take it back. Of course, finding all this took me to read the code and do some tests. You wasn't able to guess it from invoke.texi. So for this bug, I am proposing that we only give the pointless diagnostic if the flag "pedantic" is set to 1 (either by using -pedantic or -pedantic-errors).
Subject: Re: Empty macro definitions not considered equal On Mon, 26 Nov 2007, manu at gcc dot gnu dot org wrote: > ------- Comment #15 from manu at gcc dot gnu dot org 2007-11-26 17:47 ------- > (In reply to comment #14) > > > > I guess you are not explaining it wrong, but the situation is > > extremely confusing: > > > > Oh, I fully agree on that. But it is exactly the same situation that was there > pre-4.3 for the C++ front-end. It is just that now CPP and C++ front-end > defaults are consistent. Yes. > So for this bug, I am proposing that we only give the pointless diagnostic if > the flag "pedantic" is set to 1 (either by using -pedantic or > -pedantic-errors). I like this idea (and as far as I understand this is what the patch you pasted does). Thanks, Richard.
Can someone suggest a better comment for this? /* Since this diagnostic is mandated by the standard, with -pedantic we diagnose different parameters spellings even if the macro expansion has no tokens. */ The place where the standard requires this would be helpful.
The program is invalid, there is no point in getting into special cased corner cases. If you happen to have those macros in the same program from real world applications, you probably should be watching for bigger issues.