[Bug c/52283] New: "error: case label does not reduce to an integer constant" for constant folded cast expr

[Bug c/52283] New: "error: case label does not reduce to an integer constant" for constant folded cast expr

[chrbr at gcc dot gnu.org]

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

             Bug #: 52283
           Summary: "error: case label does not reduce to an integer
                    constant" for constant folded cast expr
    Classification: Unclassified
           Product: gcc
           Version: 4.6.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: chrbr@gcc.gnu.org


extern int u;

void b (int c)
{
  switch (c)
    {
    case (int) ( 2  | ( (4 < 8 ) ? 8 : u ) ) :
      break;
    }
}

fails with 
i.c:8:5: error: case label does not reduce to an integer constant

since GCC 4.6.2.

While is is legitimately a user error with respect to the standard, this brings
legacy issue for code that used to build with GCC 4.5. Note that GCC is usually
permissive with reduced integer constant use in case label, see PR39613. 

This change seems to have appeared with the removal of the lines in
c-common.c:check_case_value

  /* ??? Can we ever get nops here for a valid case value?  We
     shouldn't for C.  */
  STRIP_TYPE_NOPS (value);

so the check for INTEGER_CST now fails and fallthu into the error.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[chrbr at gcc dot gnu.org]

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

--- Comment #1 from chrbr at gcc dot gnu.org 2012-02-16 16:49:38 UTC ---
Note that the TREE_NO_WARNING is introduced in convert_to_integer:596 while
because of the unsigned-int type conversion in  build_c_cast.

A first attempt to fix is to set TREE_NO_WARNING on the constant
(!CAN_HAVE_LOCATION_P) without creating the NOP_EXPR. But Richard explained:

On 02/16/2012 01:58 PM, Richard Guenther wrote:
> You cannot have it on possibly shared tree nodes. CAN_HAVE_LOCATION_P
> tree nodes are not shared (the reverse is not true).
>

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-16 17:01:53 UTC ---
I'd say you should (when not pedantic) just attempt to handle TREE_NO_WARNING
nops in the case handling code, as if the nops weren't there.
I hope we'll eventually extend TREE_NO_WARNING to be a bit that thise tree
should be looked up in some hash table on what warnings should be suppressed on
it, so that TREE_NO_WARNING does only disable warnings that it wants to.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[joseph at codesourcery dot com]

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

--- Comment #3 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2012-02-16 17:09:30 UTC ---
On Thu, 16 Feb 2012, jakub at gcc dot gnu.org wrote:

> I hope we'll eventually extend TREE_NO_WARNING to be a bit that thise tree
> should be looked up in some hash table on what warnings should be suppressed on
> it, so that TREE_NO_WARNING does only disable warnings that it wants to.

That won't help with the issue of not being able to set it on shared 
trees, of course; you'll need separate trees for each use of a constant or 
declaration for that.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[manu at gcc dot gnu.org]

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

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

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

--- Comment #4 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-02-16 17:31:04 UTC ---
(In reply to comment #3)
> On Thu, 16 Feb 2012, jakub at gcc dot gnu.org wrote:
> 
> > I hope we'll eventually extend TREE_NO_WARNING to be a bit that thise tree
> > should be looked up in some hash table on what warnings should be suppressed on
> > it, so that TREE_NO_WARNING does only disable warnings that it wants to.
> 
> That won't help with the issue of not being able to set it on shared 
> trees, of course; you'll need separate trees for each use of a constant or 
> declaration for that.

What I don't understand is how Clang can track every constant / declaration
separately and still consume ten times less memory than GCC. What is it that
GCC is storing that takes so much space?

In any case TREE_NO_WARNING is almost always a bad idea. It is indiscriminate,
it silences any warning. It is unrelated to the warning code that it is trying
to silence, so one has to track the logs to see why it was added. It imposes an
overload that for most code is unnecessary (since most code is warning free).
And many of its uses are a hack to workaround corner cases of other hacks, when
the diagnostic code should just be smarter.

What is the problem with stripping the nops *before giving the error* and if it
still fails, give an error?

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[joseph at codesourcery dot com]

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

--- Comment #5 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2012-02-16 17:54:03 UTC ---
On Thu, 16 Feb 2012, manu at gcc dot gnu.org wrote:

> What is the problem with stripping the nops *before giving the error* and if it
> still fails, give an error?

NOPs are used to represent an intermediate expression that folded to an 
integer constant but is not an integer constant expression and cannot 
appear in one, as well as to carry TREE_NO_WARNING.  So you need to be 
careful about where you strip them to avoid making something into an 
integer constant expression that should not be one; don't strip them too 
early.

I think it would be OK to have a c_fully_fold_no_nop (which removes any 
TREE_NO_WARNING nop from the return from c_fully_fold) that is used in the 
relevant places (set_init_index, do_case, check_bitfield_type_and_width, 
build_enumerator at least) which have the pedwarn-if-pedantic logic for 
things folding to an integer constant where an integer constant expression 
is required.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[manu at gcc dot gnu.org]

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

--- Comment #6 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-02-16 18:25:30 UTC ---
(In reply to comment #5)
> On Thu, 16 Feb 2012, manu at gcc dot gnu.org wrote:
> 
> > What is the problem with stripping the nops *before giving the error* and if it
> > still fails, give an error?
> 
> NOPs are used to represent an intermediate expression that folded to an 
> integer constant but is not an integer constant expression and cannot 
> appear in one, as well as to carry TREE_NO_WARNING.  So you need to be 
> careful about where you strip them to avoid making something into an 
> integer constant expression that should not be one; don't strip them too 
> early.

Is this folding actually necessary for anything beyond diagnostics? I thought
it was agreed that folding in the FEs was EVIL and we should stop doing it.

> I think it would be OK to have a c_fully_fold_no_nop (which removes any 
> TREE_NO_WARNING nop from the return from c_fully_fold) that is used in the 
> relevant places (set_init_index, do_case, check_bitfield_type_and_width, 
> build_enumerator at least) which have the pedwarn-if-pedantic logic for 
> things folding to an integer constant where an integer constant expression 
> is required.

I wouldn't be surprised if the compiler got noticeably faster by removing all
early folding and all the workarounds and hacks that come with it, and
replacing it by doing a temporary c_fully_fold at those specific places that
needed it (and throwing away the result and keeping the original code around).

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-02-16 19:00:28 UTC ---
(In reply to comment #6)
> Is this folding actually necessary for anything beyond diagnostics? I thought
> it was agreed that folding in the FEs was EVIL and we should stop doing it.

Yes, in various places some folding is needed for language semantics, in other
places for various extensions.  The C FE does far less folding then it did.

> I wouldn't be surprised if the compiler got noticeably faster by removing all
> early folding and all the workarounds and hacks that come with it, and

The C FE these days doesn't perform the folding right away, it fully folds only
when needed (for some extension, or when passing down expressions to the
gimplifier).  And no, the C FE isn't anywhere on the radar compile time wise on
the vast majority of code (the C++ FE on the other side does still a lot of
folding (some unnecessary, some needed) early, on some sources the FE already
shows in compile time usage, but the folding in it doesn't).

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[manu at gcc dot gnu.org]

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

--- Comment #8 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-02-16 20:09:43 UTC ---
(In reply to comment #7)
> (In reply to comment #6)
> > Is this folding actually necessary for anything beyond diagnostics? I thought
> > it was agreed that folding in the FEs was EVIL and we should stop doing it.
> 
> Yes, in various places some folding is needed for language semantics, in other
> places for various extensions.  The C FE does far less folding then it did.

Talking about this particular case (folding in build_c_cast), is this actually
needed here? Couldn't GCC just delay folding until actually needed?

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[joseph at codesourcery dot com]

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

--- Comment #9 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2012-02-16 20:10:34 UTC ---
On Thu, 16 Feb 2012, manu at gcc dot gnu.org wrote:

> Is this folding actually necessary for anything beyond diagnostics? I thought
> it was agreed that folding in the FEs was EVIL and we should stop doing it.

Folding with c_fully_fold is done (a) where required for language 
semantics (in particular integer constant expressions ... remember that 
it's not possible to avoid doing struct layout quite early, for example, 
even if it would seem cleaner to have separate phases there, because 
offsetof can be used in an integer constant expression, and whether such 
an expression evaluates to 0 affects the types of other expressions), (b) 
for diagnostics (folded expressions work better in various cases that try 
to avoid diagnostics for e.g. signed/unsigned conversions that can't 
actually cause any problems), (c) in certain cases where the expression 
being built up is something such as SAVE_EXPR it would be problematic for 
the front end to try to take apart to fold inside later and (d) as a final 
lowering stage for a full expression that converts it into the form that 
will be gimplified.

> I wouldn't be surprised if the compiler got noticeably faster by removing all
> early folding and all the workarounds and hacks that come with it, and
> replacing it by doing a temporary c_fully_fold at those specific places that
> needed it (and throwing away the result and keeping the original code around).

The places that use c_fully_fold on something less than a full expression 
are doing it because it's actually needed there and removing it will break 
things.  Furthermore, if you throw away the result as you suggest, so 
repeatedly folding the same expression as a subexpression of larger 
expressions, you would be liable to make things slower.  (When 
c_fully_fold is run on something less than a full expression, the result 
is wrapped so that it *doesn't* recurse inside that subexpression when 
called on a larger expression containing it.)

For cases in (c) it may be useful to create new C-family tree codes that 
represent the operations in question at a level closer to the C source 
code, so allowing the lowering to SAVE_EXPR etc. to be delayed until a 
later c_fully_fold call.  For (b), it may be useful to create codes 
representing that a conversion was implicit, or that a diagnostic should 
be generated under certain conditions, again either processing them later 
in c_fully_fold or passing down the "diagnose this if ..." information for 
diagnostics to be generated at a later stage if the code is still 
reachable after some optimization and the compiler can't rule out whatever 
the problem is - but doing things any later than the final lowering to 
GENERIC / GIMPLE runs into the usual trade-offs between predictable 
diagnostics and avoiding diagnostics where a human can see there is no 
problem.

In any case you can't just remove the folding without creating appropriate 
new representations.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[joseph at codesourcery dot com]

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

--- Comment #10 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2012-02-16 23:52:21 UTC ---
On Thu, 16 Feb 2012, manu at gcc dot gnu.org wrote:

> Talking about this particular case (folding in build_c_cast), is this actually
> needed here? Couldn't GCC just delay folding until actually needed?

The only c_fully_fold call in build_c_cast is when a CONSTRUCTOR is 
generated from a cast to union (a GNU extension).  That's a case of 
avoiding any problems recursing later inside CONSTRUCTORs (which are not 
particularly normal expressions).  The option to delay would be to have 
some other representation of that cast which is only later lowered to a 
CONSTRUCTOR (with the inside expression being folded at that later time).

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[chrbr at gcc dot gnu.org]

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

--- Comment #11 from chrbr at gcc dot gnu.org 2012-02-20 11:58:28 UTC ---
The problem stems from the unconditional forcing of TREE_NO_WARNING in
convert_to_integer. This was done to fix PR26632. I don't see how such a
forcing can be correct, since the NO_WARNING on a constant would depend on the
fact that is casted or not, regardless of the use of the expression in the
front end.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[chrbr at gcc dot gnu.org]

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

--- Comment #12 from chrbr at gcc dot gnu.org 2012-02-20 15:27:38 UTC ---
Created attachment 26705
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26705
Tentative fix.

Testing this patch. Avoids setting TREE_NO_WARNING if not necessary, and strip
nops to check for unused values, for PR 26632.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[rguenth at gcc dot gnu.org]

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

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-02-20 15:40:44 UTC ---
Related to PR37985, which is caused by the fix for PR26632, too.  I still think
that patch should possibly be reverted instead.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[manu at gcc dot gnu.org]

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

--- Comment #14 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-02-20 15:59:39 UTC ---
(In reply to comment #13)
> Related to PR37985, which is caused by the fix for PR26632, too.  I still think
> that patch should possibly be reverted instead.

I also think that patch should be reverted, and a proper fix for that bug be
implemented instead by making the code that emits the warning smarter. I rather
have that PR26632 reopen, and no hacks, than these recent ones.

BTW, I cannot reproduce this bug in a recent trunk r184311.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[manu at gcc dot gnu.org]

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

--- Comment #15 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-02-21 09:30:11 UTC ---
Created attachment 26710
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26710
patch reverting PR26632

I am testing this. It reverts PR26632 fix and ignores recurses into NOPs at the
warning point. Do you see any potential issues?

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[joseph at codesourcery dot com]

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

--- Comment #16 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2012-02-21 17:40:36 UTC ---
On Tue, 21 Feb 2012, manu at gcc dot gnu.org wrote:

> Created attachment 26710
>   --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26710
> patch reverting PR26632
> 
> I am testing this. It reverts PR26632 fix and ignores recurses into NOPs at the
> warning point. Do you see any potential issues?

I think the full set of testcases from the patch originally proposed on 
gcc-patches should be added, but don't see any issues with this new patch 
from a front-end perspective.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[manu at gcc dot gnu.org]

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

--- Comment #17 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-02-21 17:52:12 UTC ---
(In reply to comment #16)
> I think the full set of testcases from the patch originally proposed on 
> gcc-patches should be added, but don't see any issues with this new patch 
> from a front-end perspective.

Thanks for looking at it. Sorry, I don't know which testcases you mean. Which
patch?

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[joseph at codesourcery dot com]

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

--- Comment #18 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2012-02-21 17:56:32 UTC ---
On Tue, 21 Feb 2012, manu at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52283
> 
> --- Comment #17 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-02-21 17:52:12 UTC ---
> (In reply to comment #16)
> > I think the full set of testcases from the patch originally proposed on 
> > gcc-patches should be added, but don't see any issues with this new patch 
> > from a front-end perspective.
> 
> Thanks for looking at it. Sorry, I don't know which testcases you mean. Which
> patch?

http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00794.html

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[manu at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #26710|0                           |1
        is obsolete|                            |

--- Comment #19 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-02-21 20:28:30 UTC ---
Created attachment 26718
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26718
take 2

The new testcases seem to work, but I haven't done a full check. Anyway, it
will have to wait for 4.8...

@chbr or anyone else, feel free to take it and submit it, now or later.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[manu at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |37985

--- Comment #20 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-02-21 20:29:35 UTC ---
My patch also fixes PR37985.

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[manu at gcc dot gnu.org]

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

--- Comment #21 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-02-21 20:35:31 UTC ---
BTW, why warn_if_unused_value is in stmt.c?
The comment at the top says: /* Expands front end tree to back end RTL for GCC
*/
And warn_if_unused_value is exclusively used by the C/C++ FEs. WTF!

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[chrbr at gcc dot gnu.org]

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

--- Comment #22 from chrbr at gcc dot gnu.org 2012-02-23 15:06:12 UTC ---
thanks, no regression with your patch on the 4.6 and trunk branches. OK to
commit on both ?

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[manu at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-02-24
             Blocks|                            |51937
     Ever Confirmed|0                           |1

--- Comment #23 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-02-24 00:03:31 UTC ---
(In reply to comment #22)
> thanks, no regression with your patch on the 4.6 and trunk branches. OK to
> commit on both ?

I think you need to add a Changelog and submit it to gcc-patches. Both 4.6.3
and 4.7.0 are about to be released, so you may need to wait until they are (or
not, I am not following closely the current status)

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[chrbr at gcc dot gnu.org]

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

--- Comment #24 from chrbr at gcc dot gnu.org 2012-02-27 08:25:54 UTC ---

> I think you need to add a Changelog and submit it to gcc-patches. Both 4.6.3
> and 4.7.0 are about to be released, so you may need to wait until they are (or
> not, I am not following closely the current status)

OK, do you have an entry that comes with your patch (stmt,c and convert.c) ?

The ChangeLog entry that came with the testcase was part of the initial patch:

2010-02-15  Christian Bruel  <christian.bruel@st.com>

    * gcc.dg/case-const-1.c: Add cond expr label and cast case.
    * gcc.dg/case-const-2.c: Likewise.
    * gcc.dg/case-const-3.c: Likewise.

I'll submit the whole thing to gcc-patches.

many thanks,

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[manu at gcc dot gnu.org]

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

--- Comment #25 from Manuel López-Ibáñez <manu at gcc dot gnu.org> 2012-03-16 23:26:08 UTC ---
(In reply to comment #24)
> > I think you need to add a Changelog and submit it to gcc-patches. Both 4.6.3
> > and 4.7.0 are about to be released, so you may need to wait until they are (or
> > not, I am not following closely the current status)
> 
> OK, do you have an entry that comes with your patch (stmt,c and convert.c) ?

Feel free to write whatever you like in the ChangeLog.

> I'll submit the whole thing to gcc-patches.

Thanks!

[Bug c/52283] "error: case label does not reduce to an integer constant" for constant folded cast expr

[chrbr at gcc dot gnu.org]

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

chrbr at gcc dot gnu.org changed:

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

--- Comment #26 from chrbr at gcc dot gnu.org 2012-04-19 09:15:46 UTC ---
Fixed at rev #186586