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>
Created attachment 4477 [details] Patch that reverts the part of Mark's patch that causes the new test added by the patch to fail
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
Fixed in GCC 3.4.