[Bug c++/53524] New: [4.7/4.8 Regression] Bogus and unsupressable enum comparison warning

[Bug c++/53524] New: [4.7/4.8 Regression] Bogus and unsupressable enum comparison warning

[ppluzhnikov at google dot com]

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

             Bug #: 53524
           Summary: [4.7/4.8 Regression] Bogus and unsupressable enum
                    comparison warning
    Classification: Unclassified
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: ppluzhnikov@google.com


Created attachment 27519
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27519
test case

Test case reduced from llvm/utils/TableGen/AsmMatcherEmitter.cpp

First noticed in gcc-4.7, confirmed in 4.8.0 20120515.

In template metaprogramming, it is quite common to compare enum values from
different instantiations of the same template.

GCC-4.7 started to warn about some of them. Further, the warning doesn't get
suppressed even under -Wno-enum-compare.

There is also a secondary problem: the diagnostic emitted is particularly lousy
-- it points to the enum itself, leaving no clue as to what GCC is complaining
about:

g++ -c tt.cc
tt.cc: In instantiation of ‘struct PointerUnionUIntTraits<CodeGenInstruction*,
CodeGenInstAlias*>’:
tt.cc:10:8:   required from ‘struct
PointerIntPair<PointerUnionUIntTraits<CodeGenInstruction*, CodeGenInstAlias*>
>’
tt.cc:23:9:   required from ‘class PointerUnion<CodeGenInstruction*,
CodeGenInstAlias*>’
tt.cc:27:61:   required from here
tt.cc:14:8: warning: enumeral mismatch in conditional expression:
‘PointerLikeTypeTraits<CodeGenInstruction*>::<anonymous enum>’ vs
‘PointerLikeTypeTraits<CodeGenInstAlias*>::<anonymous enum>’ [enabled by
default]
   enum {
        ^

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsupressable enum comparison warning

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-05-30 01:53:24 UTC ---
>Further, the warning doesn't get suppressed even under -Wno-enum-compare.

That is because it is not a compare; it is a conditional expression.

Also I think the warning is correct but not for a weird reason.  The type of
enum values are not finalized until after the enum is closed.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsupressable enum comparison warning

[jyasskin at gcc dot gnu.org]

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

Jeffrey Yasskin <jyasskin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jyasskin at gcc dot gnu.org

--- Comment #2 from Jeffrey Yasskin <jyasskin at gcc dot gnu.org> 2012-05-30 02:25:26 UTC ---
I think Paul's arguing that you shouldn't warn on "test ? anon_enum_1 :
anon_enum_2". The result is defined by [expr.cond]p6 to be the common type of
the two enum types (often 'int'), and if the pattern is widely used, then it's
probably not a good idea to warn about it by default.

And, of course, it's always a bad idea to have a warning that users can't turn
off.

I haven't investigated what types PT1BitsAv and PT2BitsAv are supposed to have
before the enum is closed, but if they're the same as their initializers, then
the above argument holds.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsupressable enum comparison warning

[crowl at google dot com]

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

Lawrence Crowl <crowl at google dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |crowl at google dot com

--- Comment #3 from Lawrence Crowl <crowl at google dot com> 2012-05-30 04:24:24 UTC ---
I believe the code is well-formed.  The enum in the PointerUnionUIntTraits
declaration is a plain enum, so the underlying type is not fixed.  For enums
where the underlying type is not fixed, the underlying type of each enumerator
is determined by its initializing expression, not by the underlying type of the
enumeration as a whole, until the enumeration is complete.  So, the underlying
type of each enum value is defined before the next enum value.

That two-phase typing seems to me to be a necessary consequence of the wording
in the standard.  The text of the standard could be clearer in this respect. 
In the following quote from the standard, I have added an editorial correction
that may clarify.  Note that this two-phase typing will disappear by the end of
the enumeration definition.


7.2/5

...  Following the closing brace of an enum-specifier, each enumerator has the
type of its enumeration.  ...  If the underlying type is not fixed, the
[underlying] type of each enumerator [before the closing brace] is the type of
its initializing value:
  -- If an initializer is specified for an enumerator, the initializing value
has the same type as the expression and the constant-expression shall be an
integral constant expression (5.19).
  -- If no initializer is specified for the first enumerator, the initializing
value has an unspecified integral type.
  -- Otherwise the type of the initializing value is the same as the type of
the initializing value of the preceding enumerator unless the incremented value
is not representable in that type, in which case the type is an unspecified
integral type sufficient to contain the incremented value.  If no such type
exists, the program is ill-formed.

7.2/6

For an enumeration whose underlying type is not fixed, the underlying type is
an integral type that can represent all the enumerator values defined in the
enumeration.  If no integral type can represent all the enumerator values, the
enumeration is ill-formed.  It is implementation-defined which integral type is
used as the underlying type except that the underlying type shall not be larger
than int unless the value of an enumerator cannot fit in an int or unsigned
int.  If the enumerator-list is empty, the underlying type is as if the
enumeration had a single enumerator with value 0.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsupressable enum comparison warning

[manu at gcc dot gnu.org]

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

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-05-30
                 CC|                            |manu at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #4 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-05-30 07:57:59 UTC ---
That input_location is pointing to something so bogus makes me think that
something weird is going on. In any case, it would be interesting to know why
same_type fails a few lines above, and whether it is true that g++ is not doing
the two-phase typing properly.

The following also warns, so the type of the enum values does not seem to be
the issue:

    N2 = 0 ? PointerLikeTypeTraits < PT1 >::NumLowBitsAvailable :
PointerLikeTypeTraits < PT2 >::NumLowBitsAvailable,

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[paolo.carlini at oracle dot com]

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

--- Comment #5 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-05-30 08:52:09 UTC ---
Note that in any case in current mainline the location is exactly the '?' of
the conditional expression: maybe the error message doesn't make sense but
lately we are making progress on the locations ;)

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
            Version|unknown                     |4.7.1
   Target Milestone|---                         |4.7.1

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot       |paolo.carlini at oracle dot
                   |gnu.org                     |com

--- Comment #6 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-05-30 10:22:33 UTC ---
The warnings are an unintended effect of my fix for PR16603.

We warn at the end of the below lines of call.c. At the moment isn't clear to
me *when* it would actually make sense to warn. Hints?

///////////////////////////////////

  /* [expr.cond]

     After those conversions, one of the following shall hold:

     --The second and third operands have the same type; the result  is  of
       that type.  */
  if (same_type_p (arg2_type, arg3_type))
    result_type = arg2_type;
  /* [expr.cond]

     --The second and third operands have arithmetic or enumeration
       type; the usual arithmetic conversions are performed to bring
       them to a common type, and the result is of that type.  */
  else if ((ARITHMETIC_TYPE_P (arg2_type)
        || UNSCOPED_ENUM_P (arg2_type))
       && (ARITHMETIC_TYPE_P (arg3_type)
           || UNSCOPED_ENUM_P (arg3_type)))
    {
      /* In this case, there is always a common type.  */
      result_type = type_after_usual_arithmetic_conversions (arg2_type,
                                 arg3_type);
      do_warn_double_promotion (result_type, arg2_type, arg3_type,
                "implicit conversion from %qT to %qT to "
                "match other result of conditional",
                input_location);

      if (TREE_CODE (arg2_type) == ENUMERAL_TYPE
      && TREE_CODE (arg3_type) == ENUMERAL_TYPE)
        {
          if (complain & tf_warning)
            warning (0, 
                     "enumeral mismatch in conditional expression: %qT vs %qT",
                     arg2_type, arg3_type);
        }

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gdr@integrable-solutions.ne
                   |                            |t

--- Comment #7 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-05-30 11:06:20 UTC ---
Let's add in CC Gaby, in the testsuite I see the warning triggering outside
templates for a testcase coming from a bug report of him,
g++.old-deja/g++.other/cond5.C, we have:

enum E1 {e1 = -1};
enum E2 {e2 = -1};

  int j;

  j = (i ? e1 : e2);    // { dg-warning "mismatch" }

Shall we not warn by default? Shall we give the warning a name (which?) and add
it to -Wall? To -Wextra? Neither?

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[manu at gcc dot gnu.org]

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

--- Comment #8 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-05-30 16:29:08 UTC ---
(In reply to comment #6)
> The warnings are an unintended effect of my fix for PR16603.

So, before your fix, same_type_p was returning true? 

enum { e1 = -1 };
enum { e2 = -1 };

int j;
void foo(void) {
  j = 0 ? e1 : e2;
}

g++ 4.3.2
tt.cc:6: warning: enumeral mismatch in conditional expression: ‘<anonymous
enum>’ vs ‘<anonymous enum>’

So, the warning is intended, and the fact that we did not warn before was a
bug. However, it is not clear to me how this code can be dangerous. But the
warning is very very old.

r29056 | nathan | 1999-09-02 11:21:42 +0200 (Thu, 02 Sep 1999) | 4 lines

        * call.c (build_conditional_expr): Warn on enum mismatches.
        (convert_arg_to_ellipsis): Move non-pod check to after
        conversion.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[paolo.carlini at oracle dot com]

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

--- Comment #9 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-05-30 17:22:45 UTC ---
I think there is a largely linguistic misunderstanding: when I said unintended
I meant that I did not *anticipate* that after my patch, which was fixing a
real bug, we would end up warning more, in templates too. For sure the testcase
I mentioned in my last message pre existed and likewise the code I posted here.
In my opinion the warning, by itself, definitely makes sense in general, but it
looks like, we may not want it uncinditionally, we may want to control it,
otherwise can be annoying in the template contexts mentioned here. Do you agree
with my summary? In practice: shall we give the warning a name and a switch?

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[crowl at google dot com]

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

--- Comment #10 from Lawrence Crowl <crowl at google dot com> 2012-05-30 17:37:37 UTC ---
(In reply to comment #6)
> The warnings are an unintended effect of my fix for PR16603.
> 
> We warn at the end of the below lines of call.c. At the moment isn't clear to
> me *when* it would actually make sense to warn. Hints?
> 
> ///////////////////////////////////
> 
>   /* [expr.cond]
> 
>      After those conversions, one of the following shall hold:
> 
>      --The second and third operands have the same type; the result  is  of
>        that type.  */
>   if (same_type_p (arg2_type, arg3_type))
>     result_type = arg2_type;
>   /* [expr.cond]
> 
>      --The second and third operands have arithmetic or enumeration
>        type; the usual arithmetic conversions are performed to bring
>        them to a common type, and the result is of that type.  */
>   else if ((ARITHMETIC_TYPE_P (arg2_type)
>         || UNSCOPED_ENUM_P (arg2_type))
>        && (ARITHMETIC_TYPE_P (arg3_type)
>            || UNSCOPED_ENUM_P (arg3_type)))
>     {
>       /* In this case, there is always a common type.  */
>       result_type = type_after_usual_arithmetic_conversions (arg2_type,
>                                  arg3_type);
>       do_warn_double_promotion (result_type, arg2_type, arg3_type,
>                 "implicit conversion from %qT to %qT to "
>                 "match other result of conditional",
>                 input_location);
> 
>       if (TREE_CODE (arg2_type) == ENUMERAL_TYPE
>       && TREE_CODE (arg3_type) == ENUMERAL_TYPE)
>         {
>           if (complain & tf_warning)
>             warning (0, 
>                      "enumeral mismatch in conditional expression: %qT vs %qT",
>                      arg2_type, arg3_type);
>         }

In order to get this error, both arg2 and arg3 must be interpreted as enum
types.  However, in the testcase, we are still in the enum definition, and
we are referring to enumerators in that definition, so at that point the
types of the arguments should be some form of int.  That is, the compiler
should either change the types of the enumerators at the closing brace, or
look through the enumerator's enum type to its underlying type while in
the definition that encloses the enumerator.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[crowl at google dot com]

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

--- Comment #11 from Lawrence Crowl <crowl at google dot com> 2012-05-30 17:42:14 UTC ---
(In reply to comment #7)
> Let's add in CC Gaby, in the testsuite I see the warning triggering outside
> templates for a testcase coming from a bug report of him,
> g++.old-deja/g++.other/cond5.C, we have:
> 
> enum E1 {e1 = -1};
> enum E2 {e2 = -1};
> 
>   int j;
> 
>   j = (i ? e1 : e2);    // { dg-warning "mismatch" }
> 
> Shall we not warn by default? Shall we give the warning a name (which?) and add
> it to -Wall? To -Wextra? Neither?

I believe this testcase is different and the warning is correct.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[manu at gcc dot gnu.org]

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

--- Comment #12 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-05-30 17:43:38 UTC ---
(In reply to comment #9)
> I think there is a largely linguistic misunderstanding: when I said unintended
> I meant that I did not *anticipate* that after my patch, which was fixing a
> real bug, we would end up warning more, in templates too. For sure the testcase
> I mentioned in my last message pre existed and likewise the code I posted here.
> In my opinion the warning, by itself, definitely makes sense in general, but it
> looks like, we may not want it uncinditionally, we may want to control it,
> otherwise can be annoying in the template contexts mentioned here. Do you agree
> with my summary? In practice: shall we give the warning a name and a switch?

Well, personally I think all warnings should have a switch, -Wenum-mismatch ? 

But I don't see why the fact that it is triggered within a template makes a
difference:
enum { e1 };
enum { e2 };

enum {
  a1 = e1,
  a2 = e2,
  a3 = 0 ? a1 : a2,
  a4 = 0 ? e1 : e2,
};

int j;
void foo(void) {
  j = a3;
}

I think what we don't want to warn for a3 but we want to warn for a4 because we
assume that a1 and a2 have the same type (despite the standard seems to say
that they don't until the closing brace) or we assume that the user does not
care.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[paolo.carlini at oracle dot com]

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

--- Comment #13 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-05-30 17:55:07 UTC ---
Thanks Manuel and Lawrence. If I understand correctly what L said, the simpler
and more urgent thing to do is making the warning code a bit smarter, I'll try
to do that.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org

--- Comment #14 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-05-31 00:58:14 UTC ---
I'm reviewing the whole thing. To summarize my understanding:

When we emit the warning, arg2_type and arg3_type are the types of arg2 and
arg3, thus, post PR16603, exactly the types of the two initializing
expressions, because we are still defining the enumerator NumLowBitsAvailable
of the same enum and the enum is not complete. And indeed, those types are
*different* as the warning says. Thus, it seems to me, the warning is behaving
as designed, just, post PR16603, it triggers also while we are defining
individual enumerators basing on other enumerators of the same enum. Of course
this didn't happen before PR16603 because we weren't honoring the two-phase
typing mechanism. Then it seems to me that we have nothing to strictly-speaking
"fix", but only to agree on how we want to put the warning under control. I'm
tempted to propose again just to add a -Wenum-mismatch, I note that the EDG
front-end doesn't warn with -Wall for the reference very simple case discussed
in Comments #7 and #8. I *do* understand that ideally we would like to tell the
code in build_conditional_expr_1: "hey we are comparing the types of the
initializing expressions of two enumerators of the same enum, which are
different, but the difference will go away at the end of the enum when we'll
have a single underlying type, thus please don't warn now" but I don't see a
simple way to do this: if, for example, we just compare underlying types, we
suppress a lot of other warnings, like the one in Comment #7. Given what I see
for EDG (what about CLANG?), I'm not sure we should spend a lot of time right
now tuning the mechanism of the warning itself.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[crowl at google dot com]

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

--- Comment #15 from Lawrence Crowl <crowl at google dot com> 2012-05-31 17:18:49 UTC ---
> When we emit the warning, arg2_type and arg3_type are the types
> of arg2 and arg3, thus, post PR16603, exactly the types of the
> two initializing expressions, because we are still defining the
> enumerator NumLowBitsAvailable of the same enum and the enum
> is not complete. And indeed, those types are *different* as
> the warning says.

The types of PointerLikeTypeTraits < PT1 >::NumLowBitsAvailable
and PointerLikeTypeTraits < PT2 >::NumLowBitsAvailable are enums,
but the types of PT1BitsAv and PT2BitsAv are the underlying type,
some form of int.  The normal integral promotion should provide
the ?: result type.

That is, we cannot just copy the type of the initializer for
PT1BitsAv, we need to determine the underlying type corresponding
to the converted value.

> Thus, it seems to me, the warning is behaving as designed, just,
> post PR16603, it triggers also while we are defining individual
> enumerators basing on other enumerators of the same enum.

I agree that the basic warning is right.  I am not convinced that
the enumerator types are handled as per the standard.

> Of course this didn't happen before PR16603 because we weren't
> honoring the two-phase typing mechanism. Then it seems to me that
> we have nothing to strictly-speaking "fix", but only to agree
> on how we want to put the warning under control.  I'm tempted to
> propose again just to add a -Wenum-mismatch, I note that the EDG
> front-end doesn't warn with -Wall for the reference very simple
> case discussed in Comments #7 and #8. I *do* understand that
> ideally we would like to tell the code in build_conditional_expr_1:
> "hey we are comparing the types of the initializing expressions
> of two enumerators of the same enum, which are different, but the
> difference will go away at the end of the enum when we'll have a
> single underlying type, thus please don't warn now" but I don't
> see a simple way to do this: if, for example, we just compare
> underlying types, we suppress a lot of other warnings, like the
> one in Comment #7. Given what I see for EDG (what about CLANG?),
> I'm not sure we should spend a lot of time right now tuning the
> mechanism of the warning itself.

I see two approaches.  First as you suggest above, which requires
passing context into the comparison.  Second, set each enumerator
type at the point of its definition to its unique underlying type,
and at the end of the enumeration, update all the enumerators to
have the enumeration's underlying type.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[paolo.carlini at oracle dot com]

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

--- Comment #16 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-05-31 17:48:06 UTC ---
In any case I recommend not mixing together here different issues: the first
one, subject of this report, is overeager warning in open enum context with
conditional operators (I understand that by now we agree that the warning is
behaving as designed, only it seems sensible to have a way to at least
controlling it, cmp C front end, EDG); the second one is about speculative
defects in the way the C++ front end handles enumerated types. For the latter
we need testcases in Bugzilla, actually a separate PR should be opened.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[crowl at google dot com]

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

--- Comment #17 from Lawrence Crowl <crowl at google dot com> 2012-05-31 21:08:51 UTC ---
> In any case I recommend not mixing together here different issues:
> the first one, subject of this report, is overeager warning in open
> enum context with conditional operators (I understand that by now
> we agree that the warning is behaving as designed, only it seems
> sensible to have a way to at least controlling it, cmp C front end,
> EDG); the second one is about speculative defects in the way the
> C++ front end handles enumerated types. For the latter we need
> testcases in Bugzilla, actually a separate PR should be opened.

I think this bug report is on how the C++ front end handles enums.
Its test case is very focussed on the problem.  The other test
cases WRT this error message seem to be working correctly.  So,
I think any new PR should be an RFE for the warning option.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[paolo.carlini at oracle dot com]

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

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|paolo.carlini at oracle dot |unassigned at gcc dot
                   |com                         |gnu.org

--- Comment #18 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-05-31 21:17:29 UTC ---
Really? If that's the case, I'm learning that now, then I don't think I can
help.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[ppluzhnikov at google dot com]

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

--- Comment #19 from Paul Pluzhnikov <ppluzhnikov at google dot com> 2012-05-31 21:33:51 UTC ---
(In reply to comment #16)
> In any case I recommend not mixing together here different issues:

I don't believe there are separate issues here.

> the first
> one, subject of this report, is overeager warning in open enum context with
> conditional operators

Yes, that warning is overeager *in that context*, i.e. it fires on correct
code. Further, that code is quite common in template metaprogramming,
and we've had it fire in several different packages.

> (I understand that by now we agree that the warning is
> behaving as designed,

I don't believe we agree on that.

The warning itself is good (behaving as designed) in this context:

  enum { A };
  enum { B };

  return 0 ? A : B;  // bug

I wouldn't want to suppress that warning above.

But I would *have* to suppress it (we build with -Werror), if it also fires
in the open enum context, where it does *not* behave as designed.

And that would be quite undesireable.

> only it seems sensible to have a way to at least
> controlling it, cmp C front end, EDG); the second one is about speculative
> defects in the way the C++ front end handles enumerated types. For the latter
> we need testcases in Bugzilla, actually a separate PR should be opened.

The second issue you are referring to is really just an explanation of
why the code that generates the warning is doing so: it has been given
bad data.

In comment#15, Lawrence suggested an approach ("Second, ...") that would
avoid giving bad data to build_conditional_expr_1().

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[paolo at gcc dot gnu.org]

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

--- Comment #20 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2012-06-04 19:27:26 UTC ---
Author: paolo
Date: Mon Jun  4 19:27:12 2012
New Revision: 188204

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188204
Log:
2012-06-04  Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/53524
    * doc/invoke.texi (Wenum-compare): Update documentation.

/cp
2012-06-04  Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/53524
    * call.c (build_conditional_expr_1): Use OPT_Wenum_compare
    to control enumeral mismatch in conditional expression too.

/testsuite
2012-06-04  Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/53524
    * g++.dg/warn/Wenum-compare-no-2: New.

Added:
    trunk/gcc/testsuite/g++.dg/warn/Wenum-compare-no-2.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/doc/invoke.texi
    trunk/gcc/testsuite/ChangeLog

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[paolo at gcc dot gnu.org]

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

--- Comment #21 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2012-06-04 20:31:04 UTC ---
Author: paolo
Date: Mon Jun  4 20:30:59 2012
New Revision: 188207

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188207
Log:
2012-06-04  Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/53524
    * doc/invoke.texi (Wenum-compare): Update documentation.

/cp
2012-06-04  Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/53524
    * call.c (build_conditional_expr_1): Use OPT_Wenum_compare
    to control enumeral mismatch in conditional expression too.

/testsuite
2012-06-04  Paolo Carlini  <paolo.carlini@oracle.com>

    PR c++/53524
    * g++.dg/warn/Wenum-compare-no-2: New.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/g++.dg/warn/Wenum-compare-no-2.C
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/cp/ChangeLog
    branches/gcc-4_7-branch/gcc/cp/call.c
    branches/gcc-4_7-branch/gcc/doc/invoke.texi
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[rguenth at gcc dot gnu.org]

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

--- Comment #22 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-13 13:49:34 UTC ---
Was this fixed?

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[paolo.carlini at oracle dot com]

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

--- Comment #23 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-06-13 14:01:10 UTC ---
Richard: http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00236.html

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.1                       |4.7.2

--- Comment #24 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-06-14 08:10:57 UTC ---
GCC 4.7.1 is being released, adjusting target milestone.

[Bug c++/53524] [4.7/4.8 Regression] Bogus and unsuppressible enum comparison warning

[ppluzhnikov at google dot com]

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

--- Comment #25 from Paul Pluzhnikov <ppluzhnikov at google dot com> 2012-06-19 21:56:31 UTC ---
Google ref: b/6695435

[Bug c++/53524] [4.7/4.8 Regression] Bogus enum comparison warning

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.7/4.8 Regression] Bogus  |[4.7/4.8 Regression] Bogus
                   |and unsuppressible enum     |enum comparison warning
                   |comparison warning          |

--- Comment #26 from Jason Merrill <jason at gcc dot gnu.org> 2012-06-25 15:50:57 UTC ---
Adjusting summary now that the warning is suppressible.

[Bug c++/53524] [4.7/4.8 Regression] Bogus enum comparison warning

[jason at gcc dot gnu.org]

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

--- Comment #27 from Jason Merrill <jason at gcc dot gnu.org> 2012-07-02 19:14:29 UTC ---
Author: jason
Date: Mon Jul  2 19:14:23 2012
New Revision: 189174

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189174
Log:
    PR c++/53524
gcc/cp/
    * call.c (build_conditional_expr_1): Don't warn about comparison of
    two enumerators before their enumeration is complete.
    (build_new_op_1): Call decay_conversion before warn_logical_operator.
    * decl.c (build_enumerator): Set DECL_CONTEXT of an enumerator to
    its enumeration.
    * decl2.c (mark_used): Call used_types_insert for enums.
    * semantics.c (finish_id_expression): Don't decay CONST_DECL.
    (finish_member_declaration): Don't change DECL_CONTEXT of enumerators.
    * class.c (check_field_decls): Don't change DECL_CONTEXT of enums.
    * typeck.c (convert_for_assignment): Don't decay CONST_DECL.
    (build_class_member_access_expr): Look through unscoped enums.
    * search.c (context_for_name_lookup): Look through unscoped enums.
    * pt.c (tsubst_copy_and_build): Don't decay CONST_DECL.
    (tsubst_copy): Use DECL_CONTEXT to find the enumeration.
    * tree.c (decl_linkage): Likewise.
    * cvt.c (ocp_convert): Check decayed expr for enum range warning.
gcc/c-family/
    * c-common.c (get_priority): Call default_conversion.

Added:
    trunk/gcc/testsuite/g++.dg/template/enum7.C
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/cp/class.c
    trunk/gcc/cp/cvt.c
    trunk/gcc/cp/decl.c
    trunk/gcc/cp/decl2.c
    trunk/gcc/cp/pt.c
    trunk/gcc/cp/search.c
    trunk/gcc/cp/semantics.c
    trunk/gcc/cp/tree.c
    trunk/gcc/cp/typeck.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/cpp0x/scoped_enum.C
    trunk/gcc/testsuite/g++.dg/other/ptrmem10.C
    trunk/gcc/testsuite/g++.dg/other/ptrmem11.C

[Bug c++/53524] [4.7/4.8 Regression] Bogus enum comparison warning

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #27 from Jason Merrill <jason at gcc dot gnu.org> 2012-07-02 19:14:29 UTC ---
Author: jason
Date: Mon Jul  2 19:14:23 2012
New Revision: 189174

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189174
Log:
    PR c++/53524
gcc/cp/
    * call.c (build_conditional_expr_1): Don't warn about comparison of
    two enumerators before their enumeration is complete.
    (build_new_op_1): Call decay_conversion before warn_logical_operator.
    * decl.c (build_enumerator): Set DECL_CONTEXT of an enumerator to
    its enumeration.
    * decl2.c (mark_used): Call used_types_insert for enums.
    * semantics.c (finish_id_expression): Don't decay CONST_DECL.
    (finish_member_declaration): Don't change DECL_CONTEXT of enumerators.
    * class.c (check_field_decls): Don't change DECL_CONTEXT of enums.
    * typeck.c (convert_for_assignment): Don't decay CONST_DECL.
    (build_class_member_access_expr): Look through unscoped enums.
    * search.c (context_for_name_lookup): Look through unscoped enums.
    * pt.c (tsubst_copy_and_build): Don't decay CONST_DECL.
    (tsubst_copy): Use DECL_CONTEXT to find the enumeration.
    * tree.c (decl_linkage): Likewise.
    * cvt.c (ocp_convert): Check decayed expr for enum range warning.
gcc/c-family/
    * c-common.c (get_priority): Call default_conversion.

Added:
    trunk/gcc/testsuite/g++.dg/template/enum7.C
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/cp/class.c
    trunk/gcc/cp/cvt.c
    trunk/gcc/cp/decl.c
    trunk/gcc/cp/decl2.c
    trunk/gcc/cp/pt.c
    trunk/gcc/cp/search.c
    trunk/gcc/cp/semantics.c
    trunk/gcc/cp/tree.c
    trunk/gcc/cp/typeck.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/cpp0x/scoped_enum.C
    trunk/gcc/testsuite/g++.dg/other/ptrmem10.C
    trunk/gcc/testsuite/g++.dg/other/ptrmem11.C

--- Comment #28 from Jason Merrill <jason at gcc dot gnu.org> 2012-07-02 19:24:11 UTC ---
Warning can be disabled in 4.7.1 with -Wno-enum-compare, is fixed in 4.8.0.