Bug 11667 - [3.4 Regression] wider-than-int enums never compare equal to 0
Summary: [3.4 Regression] wider-than-int enums never compare equal to 0
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 3.4.0
: P2 normal
Target Milestone: 3.4.0
Assignee: Mark Mitchell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-07-25 02:48 UTC by Alexandre Oliva
Modified: 2004-04-21 01:19 UTC (History)
1 user (show)

See Also:
Host: an
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2003-07-25 02:50:56


Attachments
Patch that reverts the part of Mark's patch that causes the new test added by the patch to fail (920 bytes, patch)
2003-07-25 02:49 UTC, Alexandre Oliva
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandre Oliva 2003-07-25 02:48:22 UTC
Mark,

Your patch for PR 10796, that implemented C++ DR377, also contained a
code clean-up that was actually wrong.  The underlying type of an enum
must be an integral type, but, after your patch, we end up with a type
whose precision is given by the min and max values of the enumeration,
that doesn't necessarily match the precision of any of the integral
types.

Richard Sandiford found out we were miscompiling libstdc++ because of
this, and I turned the code snippet he extracted from libstdc++ into
the attached testcase.  Richard also analyzed the problem in detail,
but it turned out that he didn't know for sure what had been decided
for DR377, so I ended up taking the issue over and found out the exact
bits of Mark's patch that introduced the problem but didn't contribute
to the implementation of the DR or the fix to the PR.

<quote author="Richard Sandiford">
When compiling "gx == 0", we call shorten_compare with:

    op0 = <nop_expr type <integer_type SI> arg 0 <var_decl gx>>
    op1 = <integer_cst type <int> constant 0>
    result_type = <integer_type SI>

The first thing shorten_compare does is to remove integer promotions, so:

    primop0 = <var_decl gx>
    primop1 = <integer_cst type <int> constant 0>

We then have:

  /* If comparing an integer against a constant more bits wide,
     maybe we can deduce a value of 1 or 0 independent of the data.
     Or else truncate the constant now
     rather than extend the variable at run time.

     This is only interesting if the constant is the wider arg.
     Also, it is not safe if the constant is unsigned and the
     variable arg is signed, since in this case the variable
     would be sign-extended and then regarded as unsigned.
     Our technique fails in this case because the lowest/highest
     possible unsigned results don't follow naturally from the
     lowest/highest possible values of the variable operand.
     For just EQ_EXPR and NE_EXPR there is another technique that
     could be used: see if the constant can be faithfully represented
     in the other operand's type, by truncating it and reextending it
     and see if that preserves the constant's value.  */

  if (!real1 && !real2
      && TREE_CODE (primop1) == INTEGER_CST
      && TYPE_PRECISION (TREE_TYPE (primop0)) < TYPE_PRECISION (*restype_ptr))
    {
      int min_gt, max_gt, min_lt, max_lt;
      tree maxval, minval;
      /* 1 if comparison is nominally unsigned.  */
      int unsignedp = TREE_UNSIGNED (*restype_ptr);
      tree val;

      type = c_common_signed_or_unsigned_type (unsignedp0,
					       TREE_TYPE (primop0));

      /* If TYPE is an enumeration, then we need to get its min/max
	 values from it's underlying integral type, not the enumerated
	 type itself.  */
      if (TREE_CODE (type) == ENUMERAL_TYPE)
--->	type = c_common_type_for_size (TYPE_PRECISION (type), unsignedp0);

Before Mark's patch, TYPE_PRECISION (TREE_TYPE (gx)) was 32, so this
code wasn't executed.  But now gx's precision is 17.

At the indicated line, "type" becomes the unsigned version of the
underlying type (i.e. an unsigned SI type).  We then convert the
minimum and maximum values of this type into the "common type",
which is signed in this case:

      maxval = TYPE_MAX_VALUE (type);
      minval = TYPE_MIN_VALUE (type);
      [...]
      if (type != *restype_ptr)
	{
	  minval = convert (*restype_ptr, minval);
	  maxval = convert (*restype_ptr, maxval);
	}

maxval is now -1 and any non-negative number seems out of range.
</quote>
Comment 1 Alexandre Oliva 2003-07-25 02:49:44 UTC
Created attachment 4477 [details]
Patch that reverts the part of Mark's patch that causes the new test added by the patch to fail
Comment 2 GCC Commits 2003-07-29 01:14:30 UTC
Subject: Bug 11667

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	mmitchel@gcc.gnu.org	2003-07-29 01:14:25

Modified files:
	gcc            : ChangeLog c-common.c stor-layout.c tree.h 
	gcc/cp         : ChangeLog Make-lang.in call.c class.c decl.c 
	                 pt.c rtti.c typeck.c 
	gcc/testsuite  : ChangeLog 
	gcc/testsuite/g++.dg/template: overload1.C 
Added files:
	gcc/testsuite/g++.dg/init: enum2.C 

Log message:
	PR c++/11667
	* c-common.c (shorten_compare): Take into account differences
	between C and C++ representation for enumeration types.
	* tree.h (set_min_and_max_values_for_integral_type): Declare.
	* stor-layout.c (set_min_and_max_values_for_integral_type): New
	function, broken out from ...
	(fixup_signed_type): ... here and ...
	(fixup_unsigned_type): ... here.
	
	PR c++/11667
	* call.c (standard_conversion): Allow all integral->enumeral
	conversions, after marking them as bad.
	* decl.c (finish_enum): Make sure that all enumerators are
	properly converted to the underlying type.
	(build_enumerator): Set DECL_CONTEXT for namespace-scope
	enumeration types.
	* pt.c (tsubst_copy): Adjust handling of CONST_DECLs accordingly.
	(tsubst_enum): Tidy.
	
	* Make-lang.in (typeck.o): Depend on convert.h.
	(class.o): Likewise.
	(rtti.o): Likewise.
	* call.c: Include convert.h.
	(convert_arg_to_ellipsis): Use convert_to_real.
	* class.c: Include convert.h.
	(build_base_path): Use convert_to_integer.
	* rtti.c: Include convert.h.
	(build_headof): Use convert_to_integer.
	* typeck.c: Include convert.h.
	(decay_conversion): Use convert_to_integer.
	(build_unary_op): Use build_nop.
	(get_delta_difference): Use convert_to_integer.
	(build_ptrmemfunc): Avoid unncessary conversions.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.641&r2=2.642
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/c-common.c.diff?cvsroot=gcc&r1=1.439&r2=1.440
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/stor-layout.c.diff?cvsroot=gcc&r1=1.164&r2=1.165
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree.h.diff?cvsroot=gcc&r1=1.430&r2=1.431
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.3563&r2=1.3564
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/Make-lang.in.diff?cvsroot=gcc&r1=1.158&r2=1.159
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&r1=1.416&r2=1.417
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/class.c.diff?cvsroot=gcc&r1=1.557&r2=1.558
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/decl.c.diff?cvsroot=gcc&r1=1.1102&r2=1.1103
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/pt.c.diff?cvsroot=gcc&r1=1.740&r2=1.741
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/rtti.c.diff?cvsroot=gcc&r1=1.170&r2=1.171
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/typeck.c.diff?cvsroot=gcc&r1=1.487&r2=1.488
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.2925&r2=1.2926
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/init/enum2.C.diff?cvsroot=gcc&r1=1.1&r2=1.2
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/template/overload1.C.diff?cvsroot=gcc&r1=1.1&r2=1.2

Comment 3 Mark Mitchell 2003-07-29 01:18:01 UTC
Fixed in GCC 3.4.