[Bug preprocessor/33907] New: Empty macro definitions not considered equal

[Bug preprocessor/33907] New: Empty macro definitions not considered equal

[rguenth at gcc dot gnu dot org]

#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])


-- 
           Summary: Empty macro definitions not considered equal
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: diagnostic
          Severity: normal
          Priority: P3
         Component: preprocessor
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rguenth at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[schwab at suse dot de]

------- Comment #1 from schwab at suse dot de  2007-10-26 13:44 -------
The standard requires the spelling of parameters to be the same even if the
replacement list is empty.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2007-10-26 13:46 -------
I know - I should have made this an enhancement request.  Certainly there
will be no observable difference for empty replacement lists, no?


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[schwab at suse dot de]

------- Comment #3 from schwab at suse dot de  2007-10-26 13:55 -------
The difference is that it is a constraint.  You must diagnose it at least in
pedantic mode.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[joseph at codesourcery dot com]

------- Comment #4 from joseph at codesourcery dot com  2007-10-26 14:45 -------
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).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[tromey at gcc dot gnu dot org]

------- 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?


-- 

tromey at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[rguenther at suse dot de]

------- Comment #6 from rguenther at suse dot de  2007-11-25 21:34 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[manu at gcc dot gnu dot org]

------- 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. 

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)


-- 

manu at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu dot org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2007-11-25 22:23:50
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[rguenther at suse dot de]

------- Comment #8 from rguenther at suse dot de  2007-11-25 22:29 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[manu at gcc dot gnu dot org]

------- Comment #9 from manu at gcc dot gnu dot org  2007-11-25 22:56 -------
(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?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[manu at gcc dot gnu dot org]

------- 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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[rguenther at suse dot de]

------- Comment #11 from rguenther at suse dot de  2007-11-26 09:56 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[joseph at codesourcery dot com]

------- Comment #12 from joseph at codesourcery dot com  2007-11-26 12:44 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[manu at gcc dot gnu dot org]

------- 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.


-- 

manu at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2007-11-25 22:23:50         |2007-11-26 13:03:29
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[rguenther at suse dot de]

------- Comment #14 from rguenther at suse dot de  2007-11-26 13:15 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[manu at gcc dot gnu dot org]

------- 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.

> 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).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[rguenther at suse dot de]

------- Comment #16 from rguenther at suse dot de  2007-11-27 09:10 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[manu at gcc dot gnu dot org]

-- 

manu at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |manu at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2007-11-26 13:03:29         |2007-11-27 13:51:34
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[manu at gcc dot gnu dot org]

------- Comment #17 from manu at gcc dot gnu dot org  2007-11-27 13:57 -------
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.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907

[Bug preprocessor/33907] Empty macro definitions not considered equal

[gdr at gcc dot gnu dot org]

------- Comment #18 from gdr at gcc dot gnu dot org  2007-11-30 21:10 -------
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.


-- 

gdr at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |INVALID


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33907