diff options
| -rw-r--r-- | gcc/ChangeLog | 17 | ||||
| -rw-r--r-- | gcc/c-decl.c | 95 | ||||
| -rw-r--r-- | gcc/c-typeck.c | 84 | ||||
| -rw-r--r-- | gcc/testsuite/gcc.dg/compare1.c | 37 |
4 files changed, 151 insertions, 82 deletions
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 460c08b..4f8b02f 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,20 @@ +2000-01-05 11:25 -0800 Zack Weinberg <zack@rabi.columbia.edu> + + * c-decl.c (finish_enum): Simplify code to determine minimum and + maximum values of the enum, and calculate the type. Remove check + for FUNCTION_DECLs in the values list, which cannot happen. Replace + the DECL_INITIAL of each enumeration constant with a copy converted + to the enumeration type. When updating variant types, don't bother + updating the type itself. + + * c-typeck.c (build_binary_op): Simplify conditional expressions + when weeding out spurious signed-unsigned warnings. Add new + spurious warning category: if the unsigned quantity is an enum + and its maximum value fits in signed_type(result_type). Update + commentary. + (build_conditional_expr): Warn here if one alternative is signed + and the other is unsigned. + 2000-01-05 Nick Clifton <nickc@cygnus.com> * config/fr30/fr30.h: Remove extraneous comments. diff --git a/gcc/c-decl.c b/gcc/c-decl.c index 2eba0e4..850ac6d 100644 --- a/gcc/c-decl.c +++ b/gcc/c-decl.c @@ -5664,8 +5664,8 @@ finish_enum (enumtype, values, attributes) { register tree pair, tem; tree minnode = 0, maxnode = 0; - int lowprec, highprec, precision; - int toplevel = global_binding_level == current_binding_level; + int precision, unsign; + int toplevel = (global_binding_level == current_binding_level); if (in_parm_level_p ()) warning ("enum defined inside parms"); @@ -5677,67 +5677,62 @@ finish_enum (enumtype, values, attributes) if (values == error_mark_node) minnode = maxnode = integer_zero_node; else - for (pair = values; pair; pair = TREE_CHAIN (pair)) - { - tree value = TREE_VALUE (pair); - if (pair == values) - minnode = maxnode = TREE_VALUE (pair); - else - { - if (tree_int_cst_lt (maxnode, value)) - maxnode = value; - if (tree_int_cst_lt (value, minnode)) - minnode = value; - } - } - - TYPE_MIN_VALUE (enumtype) = minnode; - TYPE_MAX_VALUE (enumtype) = maxnode; - - /* An enum can have some negative values; then it is signed. */ - TREE_UNSIGNED (enumtype) = tree_int_cst_sgn (minnode) >= 0; - - /* Determine the precision this type needs. */ - - lowprec = min_precision (minnode, TREE_UNSIGNED (enumtype)); - highprec = min_precision (maxnode, TREE_UNSIGNED (enumtype)); - precision = MAX (lowprec, highprec); - - if (TYPE_PACKED (enumtype) || precision > TYPE_PRECISION (integer_type_node)) { - tree narrowest = type_for_size (precision, 1); - if (narrowest == 0) + minnode = maxnode = TREE_VALUE (values); + for (pair = TREE_CHAIN (values); pair; pair = TREE_CHAIN (pair)) { - warning ("enumeration values exceed range of largest integer"); - narrowest = long_long_integer_type_node; + tree value = TREE_VALUE (pair); + if (tree_int_cst_lt (maxnode, value)) + maxnode = value; + if (tree_int_cst_lt (value, minnode)) + minnode = value; } + } - TYPE_PRECISION (enumtype) = TYPE_PRECISION (narrowest); + /* Construct the final type of this enumeration. It is the same + as one of the integral types - the narrowest one that fits, except + that normally we only go as narrow as int - and signed iff any of + the values are negative. */ + unsign = (tree_int_cst_sgn (minnode) >= 0); + precision = MAX (min_precision (minnode, unsign), + min_precision (maxnode, unsign)); + if (!TYPE_PACKED (enumtype)) + precision = MAX (precision, TYPE_PRECISION (integer_type_node)); + if (type_for_size (precision, unsign) == 0) + { + warning ("enumeration values exceed range of largest integer"); + precision = TYPE_PRECISION (long_long_integer_type_node); } - else - TYPE_PRECISION (enumtype) = TYPE_PRECISION (integer_type_node); + TYPE_MIN_VALUE (enumtype) = minnode; + TYPE_MAX_VALUE (enumtype) = maxnode; + TYPE_PRECISION (enumtype) = precision; + TREE_UNSIGNED (enumtype) = unsign; TYPE_SIZE (enumtype) = 0; layout_type (enumtype); if (values != error_mark_node) { - /* Change the type of the enumerators to be the enum type. - Formerly this was done only for enums that fit in an int, - but the comment said it was done only for enums wider than int. - It seems necessary to do this for wide enums, - and best not to change what's done for ordinary narrower ones. */ + /* Change the type of the enumerators to be the enum type. We + need to do this irrespective of the size of the enum, for + proper type checking. Replace the DECL_INITIALs of the + enumerators, and the value slots of the list, with copies + that have the enum type; they cannot be modified in place + because they may be shared (e.g. integer_zero_node) Finally, + change the purpose slots to point to the names of the decls. */ for (pair = values; pair; pair = TREE_CHAIN (pair)) { - TREE_TYPE (TREE_PURPOSE (pair)) = enumtype; - DECL_SIZE (TREE_PURPOSE (pair)) = TYPE_SIZE (enumtype); - if (TREE_CODE (TREE_PURPOSE (pair)) != FUNCTION_DECL) - DECL_ALIGN (TREE_PURPOSE (pair)) = TYPE_ALIGN (enumtype); - } + tree enu = TREE_PURPOSE (pair); - /* Replace the decl nodes in VALUES with their names. */ - for (pair = values; pair; pair = TREE_CHAIN (pair)) - TREE_PURPOSE (pair) = DECL_NAME (TREE_PURPOSE (pair)); + TREE_TYPE (enu) = enumtype; + DECL_SIZE (enu) = TYPE_SIZE (enumtype); + DECL_ALIGN (enu) = TYPE_ALIGN (enumtype); + DECL_MODE (enu) = TYPE_MODE (enumtype); + DECL_INITIAL (enu) = convert (enumtype, DECL_INITIAL (enu)); + + TREE_PURPOSE (pair) = DECL_NAME (enu); + TREE_VALUE (pair) = DECL_INITIAL (enu); + } TYPE_VALUES (enumtype) = values; } @@ -5745,6 +5740,8 @@ finish_enum (enumtype, values, attributes) /* Fix up all variant types of this enum type. */ for (tem = TYPE_MAIN_VARIANT (enumtype); tem; tem = TYPE_NEXT_VARIANT (tem)) { + if (tem == enumtype) + continue; TYPE_VALUES (tem) = TYPE_VALUES (enumtype); TYPE_MIN_VALUE (tem) = TYPE_MIN_VALUE (enumtype); TYPE_MAX_VALUE (tem) = TYPE_MAX_VALUE (enumtype); diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c index 8b6a0d6..0529d2a 100644 --- a/gcc/c-typeck.c +++ b/gcc/c-typeck.c @@ -2390,8 +2390,6 @@ build_binary_op (code, orig_op0, orig_op1, convert_p) tree primop0 = get_narrower (op0, &unsignedp0); tree primop1 = get_narrower (op1, &unsignedp1); - /* Avoid spurious warnings for comparison with enumerators. */ - xop0 = orig_op0; xop1 = orig_op1; STRIP_TYPE_NOPS (xop0); @@ -2407,28 +2405,41 @@ build_binary_op (code, orig_op0, orig_op1, convert_p) all the values of the unsigned type. */ if (! TREE_UNSIGNED (result_type)) /* OK */; - /* Do not warn if both operands are unsigned. */ + /* Do not warn if both operands are the same signedness. */ else if (op0_signed == op1_signed) /* OK */; - /* Do not warn if the signed quantity is an unsuffixed - integer literal (or some static constant expression - involving such literals) and it is non-negative. */ - else if ((op0_signed && TREE_CODE (xop0) == INTEGER_CST - && tree_int_cst_sgn (xop0) >= 0) - || (op1_signed && TREE_CODE (xop1) == INTEGER_CST - && tree_int_cst_sgn (xop1) >= 0)) - /* OK */; - /* Do not warn if the comparison is an equality operation, - the unsigned quantity is an integral constant and it does - not use the most significant bit of result_type. */ - else if ((resultcode == EQ_EXPR || resultcode == NE_EXPR) - && ((op0_signed && TREE_CODE (xop1) == INTEGER_CST - && int_fits_type_p (xop1, signed_type (result_type))) - || (op1_signed && TREE_CODE (xop0) == INTEGER_CST - && int_fits_type_p (xop0, signed_type (result_type))))) - /* OK */; else - warning ("comparison between signed and unsigned"); + { + tree sop, uop; + if (op0_signed) + sop = xop0, uop = xop1; + else + sop = xop1, uop = xop0; + + /* Do not warn if the signed quantity is an unsuffixed + integer literal (or some static constant expression + involving such literals) and it is non-negative. */ + if (TREE_CODE (sop) == INTEGER_CST + && tree_int_cst_sgn (sop) >= 0) + /* OK */; + /* Do not warn if the comparison is an equality operation, + the unsigned quantity is an integral constant, and it + would fit in the result if the result were signed. */ + else if (TREE_CODE (uop) == INTEGER_CST + && (resultcode == EQ_EXPR || resultcode == NE_EXPR) + && int_fits_type_p (uop, signed_type (result_type))) + /* OK */; + /* Do not warn if the unsigned quantity is an enumeration + constant and its maximum value would fit in the result + if the result were signed. */ + else if (TREE_CODE (uop) == INTEGER_CST + && TREE_CODE (TREE_TYPE (uop)) == ENUMERAL_TYPE + && int_fits_type_p (TYPE_MAX_VALUE (TREE_TYPE(uop)), + signed_type (result_type))) + /* OK */; + else + warning ("comparison between signed and unsigned"); + } /* Warn if two unsigned values are being compared in a size larger than their original size, and one (and only one) is the @@ -3362,6 +3373,37 @@ build_conditional_expr (ifexp, op1, op2) && (code2 == INTEGER_TYPE || code2 == REAL_TYPE)) { result_type = common_type (type1, type2); + + /* If -Wsign-compare, warn here if type1 and type2 have + different signedness. We'll promote the signed to unsigned + and later code won't know it used to be different. + Do this check on the original types, so that explicit casts + will be considered, but default promotions won't. */ + if ((warn_sign_compare < 0 ? extra_warnings : warn_sign_compare) + && !skip_evaluation) + { + int unsigned_op1 = TREE_UNSIGNED (TREE_TYPE (orig_op1)); + int unsigned_op2 = TREE_UNSIGNED (TREE_TYPE (orig_op2)); + + if (unsigned_op1 ^ unsigned_op2) + { + /* Do not warn if the result type is signed, since the + signed type will only be chosen if it can represent + all the values of the unsigned type. */ + if (! TREE_UNSIGNED (result_type)) + /* OK */; + /* Do not warn if the signed quantity is an unsuffixed + integer literal (or some static constant expression + involving such literals) and it is non-negative. */ + else if ((unsigned_op2 && TREE_CODE (op1) == INTEGER_CST + && tree_int_cst_sgn (op1) >= 0) + || (unsigned_op1 && TREE_CODE (op2) == INTEGER_CST + && tree_int_cst_sgn (op2) >= 0)) + /* OK */; + else + warning ("signed and unsigned type in conditional expression"); + } + } } else if (code1 == VOID_TYPE || code2 == VOID_TYPE) { diff --git a/gcc/testsuite/gcc.dg/compare1.c b/gcc/testsuite/gcc.dg/compare1.c index 3be9d95..44c4df8 100644 --- a/gcc/testsuite/gcc.dg/compare1.c +++ b/gcc/testsuite/gcc.dg/compare1.c @@ -4,23 +4,36 @@ /* { dg-do compile } */ /* { dg-options "-Wsign-compare" } */ -int target_flags = 1; +int tf = 1; -enum machine_mode +/* This enumeration has an explicit negative value and is therefore signed. */ +enum mm1 { - VOIDmode , PQImode , QImode , PHImode , HImode , - PSImode , SImode , PDImode , DImode , TImode , OImode , QFmode , - HFmode , TQFmode , SFmode , DFmode , XFmode , TFmode , QCmode , - HCmode , SCmode , DCmode , XCmode , TCmode , CQImode , CHImode , - CSImode , CDImode , CTImode , COImode , BLKmode , CCmode , CCXmode, - CC_NOOVmode, CCX_NOOVmode, CCFPmode, CCFPEmode , MAX_MACHINE_MODE + VOID, SI, DI, MAX = -1 }; -#define Pmode ( target_flags ? DImode : SImode ) +/* This enumeration fits entirely in a signed int, but is unsigned anyway. */ +enum mm2 +{ + VOID2, SI2, DI2, MAX2 +}; + +int f(enum mm1 x) +{ + return x == (tf?DI:SI); /* { dg-bogus "signed and unsigned" "case 1" } */ +} -int main() +int g(enum mm1 x) { - enum machine_mode mode = DImode; + return x == (tf?DI:-1); /* { dg-bogus "signed and unsigned" "case 2" } */ +} - return (mode == Pmode); /* { dg-bogus "warning:" "comparison between signed and unsigned" } */ +int h(enum mm2 x) +{ + return x == (tf?DI2:SI2); /* { dg-bogus "signed and unsigned" "case 3" } */ +} + +int i(enum mm2 x) +{ + return x == (tf?DI2:-1); /* { dg-warning "signed and unsigned" "case 4" } */ } |