From 6715192c06501e2bc1ee1a2670697dbe5d0efd41 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Manuel=20L=C3=B3pez-Ib=C3=A1=C3=B1ez?= Date: Wed, 30 Jul 2008 08:30:32 +0000 Subject: [PATCH] re PR c/34389 (-Wconversion produces wrong warning) 2008-07-30 Manuel Lopez-Ibanez PR 34389 * c-typeck.c (build_binary_op): Encapsulate code into... * c-common.c (shorten_binary_op): ...this new function. (conversion_warning): Use the new function. Handle non-negative constant in bitwise-and. * c-common.h (shorten_binary_op): Declare. cp/ * typeck.c (build_binary_op): Encapsulate code into shorten_binary_op. testsuite/ * gcc.dg/Wconversion-pr34389.c: New. * g++.dg/warn/Wconversion-pr34389.C: New. From-SVN: r138296 --- gcc/ChangeLog | 9 + gcc/c-common.c | 161 ++++++++++++++++-- gcc/c-common.h | 3 + gcc/c-typeck.c | 88 +--------- gcc/cp/ChangeLog | 6 + gcc/cp/typeck.c | 56 +----- gcc/testsuite/ChangeLog | 6 + .../g++.dg/warn/Wconversion-pr34389.C | 53 ++++++ gcc/testsuite/gcc.dg/Wconversion-pr34389.c | 53 ++++++ 9 files changed, 282 insertions(+), 153 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C create mode 100644 gcc/testsuite/gcc.dg/Wconversion-pr34389.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 19e2f3b0f26..671b794cf25 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2008-07-30 Manuel Lopez-Ibanez + + PR 34389 + * c-typeck.c (build_binary_op): Encapsulate code into... + * c-common.c (shorten_binary_op): ...this new function. + (conversion_warning): Use the new function. Handle non-negative + constant in bitwise-and. + * c-common.h (shorten_binary_op): Declare. + 2008-07-30 Olivier Hainque * scan.c (make_sstring_space): Add explicit conversions of diff --git a/gcc/c-common.c b/gcc/c-common.c index caac53e7cdd..dac29ea2c7e 100644 --- a/gcc/c-common.c +++ b/gcc/c-common.c @@ -1447,6 +1447,110 @@ vector_types_convertible_p (const_tree t1, const_tree t2, bool emit_lax_note) return false; } +/* This is a helper function of build_binary_op. + + For certain operations if both args were extended from the same + smaller type, do the arithmetic in that type and then extend. + + BITWISE indicates a bitwise operation. + For them, this optimization is safe only if + both args are zero-extended or both are sign-extended. + Otherwise, we might change the result. + Eg, (short)-1 | (unsigned short)-1 is (int)-1 + but calculated in (unsigned short) it would be (unsigned short)-1. +*/ +tree shorten_binary_op (tree result_type, tree op0, tree op1, bool bitwise) +{ + int unsigned0, unsigned1; + tree arg0, arg1; + int uns; + tree type; + + /* Cast OP0 and OP1 to RESULT_TYPE. Doing so prevents + excessive narrowing when we call get_narrower below. For + example, suppose that OP0 is of unsigned int extended + from signed char and that RESULT_TYPE is long long int. + If we explicitly cast OP0 to RESULT_TYPE, OP0 would look + like + + (long long int) (unsigned int) signed_char + + which get_narrower would narrow down to + + (unsigned int) signed char + + If we do not cast OP0 first, get_narrower would return + signed_char, which is inconsistent with the case of the + explicit cast. */ + op0 = convert (result_type, op0); + op1 = convert (result_type, op1); + + arg0 = get_narrower (op0, &unsigned0); + arg1 = get_narrower (op1, &unsigned1); + + /* UNS is 1 if the operation to be done is an unsigned one. */ + uns = TYPE_UNSIGNED (result_type); + + /* Handle the case that OP0 (or OP1) does not *contain* a conversion + but it *requires* conversion to FINAL_TYPE. */ + + if ((TYPE_PRECISION (TREE_TYPE (op0)) + == TYPE_PRECISION (TREE_TYPE (arg0))) + && TREE_TYPE (op0) != result_type) + unsigned0 = TYPE_UNSIGNED (TREE_TYPE (op0)); + if ((TYPE_PRECISION (TREE_TYPE (op1)) + == TYPE_PRECISION (TREE_TYPE (arg1))) + && TREE_TYPE (op1) != result_type) + unsigned1 = TYPE_UNSIGNED (TREE_TYPE (op1)); + + /* Now UNSIGNED0 is 1 if ARG0 zero-extends to FINAL_TYPE. */ + + /* For bitwise operations, signedness of nominal type + does not matter. Consider only how operands were extended. */ + if (bitwise) + uns = unsigned0; + + /* Note that in all three cases below we refrain from optimizing + an unsigned operation on sign-extended args. + That would not be valid. */ + + /* Both args variable: if both extended in same way + from same width, do it in that width. + Do it unsigned if args were zero-extended. */ + if ((TYPE_PRECISION (TREE_TYPE (arg0)) + < TYPE_PRECISION (result_type)) + && (TYPE_PRECISION (TREE_TYPE (arg1)) + == TYPE_PRECISION (TREE_TYPE (arg0))) + && unsigned0 == unsigned1 + && (unsigned0 || !uns)) + return c_common_signed_or_unsigned_type + (unsigned0, common_type (TREE_TYPE (arg0), TREE_TYPE (arg1))); + + else if (TREE_CODE (arg0) == INTEGER_CST + && (unsigned1 || !uns) + && (TYPE_PRECISION (TREE_TYPE (arg1)) + < TYPE_PRECISION (result_type)) + && (type + = c_common_signed_or_unsigned_type (unsigned1, + TREE_TYPE (arg1))) + && !POINTER_TYPE_P (type) + && int_fits_type_p (arg0, type)) + return type; + + else if (TREE_CODE (arg1) == INTEGER_CST + && (unsigned0 || !uns) + && (TYPE_PRECISION (TREE_TYPE (arg0)) + < TYPE_PRECISION (result_type)) + && (type + = c_common_signed_or_unsigned_type (unsigned0, + TREE_TYPE (arg0))) + && !POINTER_TYPE_P (type) + && int_fits_type_p (arg1, type)) + return type; + + return result_type; +} + /* Warns if the conversion of EXPR to TYPE may alter a value. This is a helper function for warnings_for_convert_and_check. */ @@ -1511,42 +1615,73 @@ conversion_warning (tree type, tree expr) } else /* 'expr' is not a constant. */ { + tree expr_type = TREE_TYPE (expr); + /* Warn for real types converted to integer types. */ - if (TREE_CODE (TREE_TYPE (expr)) == REAL_TYPE + if (TREE_CODE (expr_type) == REAL_TYPE && TREE_CODE (type) == INTEGER_TYPE) give_warning = true; - else if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE + else if (TREE_CODE (expr_type) == INTEGER_TYPE && TREE_CODE (type) == INTEGER_TYPE) { /* Don't warn about unsigned char y = 0xff, x = (int) y; */ expr = get_unwidened (expr, 0); + expr_type = TREE_TYPE (expr); + /* Don't warn for short y; short x = ((int)y & 0xff); */ + if (TREE_CODE (expr) == BIT_AND_EXPR + || TREE_CODE (expr) == BIT_IOR_EXPR + || TREE_CODE (expr) == BIT_XOR_EXPR) + { + /* It both args were extended from a shortest type, use + that type if that is safe. */ + expr_type = shorten_binary_op (expr_type, + TREE_OPERAND (expr, 0), + TREE_OPERAND (expr, 1), + /* bitwise */1); + + /* If one of the operands is a non-negative constant + that fits in the target type, then the type of the + other operand does not matter. */ + if (TREE_CODE (expr) == BIT_AND_EXPR) + { + tree op0 = TREE_OPERAND (expr, 0); + tree op1 = TREE_OPERAND (expr, 1); + if ((TREE_CODE (op0) == INTEGER_CST + && int_fits_type_p (op0, c_common_signed_type (type)) + && int_fits_type_p (op0, c_common_unsigned_type (type))) + || (TREE_CODE (op1) == INTEGER_CST + && int_fits_type_p (op1, c_common_signed_type (type)) + && int_fits_type_p (op1, c_common_unsigned_type (type)))) + return; + } + } /* Warn for integer types converted to smaller integer types. */ - if (formal_prec < TYPE_PRECISION (TREE_TYPE (expr))) + if (formal_prec < TYPE_PRECISION (expr_type)) give_warning = true; /* When they are the same width but different signedness, then the value may change. */ - else if ((formal_prec == TYPE_PRECISION (TREE_TYPE (expr)) - && TYPE_UNSIGNED (TREE_TYPE (expr)) != TYPE_UNSIGNED (type)) + else if ((formal_prec == TYPE_PRECISION (expr_type) + && TYPE_UNSIGNED (expr_type) != TYPE_UNSIGNED (type)) /* Even when converted to a bigger type, if the type is unsigned but expr is signed, then negative values will be changed. */ - || (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (TREE_TYPE (expr)))) + || (TYPE_UNSIGNED (type) && !TYPE_UNSIGNED (expr_type))) warning (OPT_Wsign_conversion, "conversion to %qT from %qT may change the sign of the result", - type, TREE_TYPE (expr)); + type, expr_type); } /* Warn for integer types converted to real types if and only if all the range of values of the integer type cannot be represented by the real type. */ - else if (TREE_CODE (TREE_TYPE (expr)) == INTEGER_TYPE + else if (TREE_CODE (expr_type) == INTEGER_TYPE && TREE_CODE (type) == REAL_TYPE) { - tree type_low_bound = TYPE_MIN_VALUE (TREE_TYPE (expr)); - tree type_high_bound = TYPE_MAX_VALUE (TREE_TYPE (expr)); + tree type_low_bound = TYPE_MIN_VALUE (expr_type); + tree type_high_bound = TYPE_MAX_VALUE (expr_type); REAL_VALUE_TYPE real_low_bound = real_value_from_int_cst (0, type_low_bound); REAL_VALUE_TYPE real_high_bound = real_value_from_int_cst (0, type_high_bound); @@ -1556,16 +1691,16 @@ conversion_warning (tree type, tree expr) } /* Warn for real types converted to smaller real types. */ - else if (TREE_CODE (TREE_TYPE (expr)) == REAL_TYPE + else if (TREE_CODE (expr_type) == REAL_TYPE && TREE_CODE (type) == REAL_TYPE - && formal_prec < TYPE_PRECISION (TREE_TYPE (expr))) + && formal_prec < TYPE_PRECISION (expr_type)) give_warning = true; if (give_warning) warning (OPT_Wconversion, "conversion to %qT from %qT may alter its value", - type, TREE_TYPE (expr)); + type, expr_type); } } diff --git a/gcc/c-common.h b/gcc/c-common.h index 1ff5d665532..f600751f0c0 100644 --- a/gcc/c-common.h +++ b/gcc/c-common.h @@ -743,6 +743,9 @@ extern bool same_scalar_type_ignoring_signedness (tree, tree); #define c_sizeof(T) c_sizeof_or_alignof_type (T, true, 1) #define c_alignof(T) c_sizeof_or_alignof_type (T, false, 1) +/* Subroutine of build_binary_op, used for certain operations. */ +extern tree shorten_binary_op (tree result_type, tree op0, tree op1, bool bitwise); + /* Subroutine of build_binary_op, used for comparison operations. See if the operands have both been converted from subword integer types and, if so, perhaps change them both back to their original type. */ diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c index 160229ad8b8..4756e256f38 100644 --- a/gcc/c-typeck.c +++ b/gcc/c-typeck.c @@ -8316,93 +8316,9 @@ build_binary_op (enum tree_code code, tree orig_op0, tree orig_op1, if (shorten && none_complex) { - int unsigned0, unsigned1; - tree arg0, arg1; - int uns; - tree type; - - /* Cast OP0 and OP1 to RESULT_TYPE. Doing so prevents - excessive narrowing when we call get_narrower below. For - example, suppose that OP0 is of unsigned int extended - from signed char and that RESULT_TYPE is long long int. - If we explicitly cast OP0 to RESULT_TYPE, OP0 would look - like - - (long long int) (unsigned int) signed_char - - which get_narrower would narrow down to - - (unsigned int) signed char - - If we do not cast OP0 first, get_narrower would return - signed_char, which is inconsistent with the case of the - explicit cast. */ - op0 = convert (result_type, op0); - op1 = convert (result_type, op1); - - arg0 = get_narrower (op0, &unsigned0); - arg1 = get_narrower (op1, &unsigned1); - - /* UNS is 1 if the operation to be done is an unsigned one. */ - uns = TYPE_UNSIGNED (result_type); - final_type = result_type; - - /* Handle the case that OP0 (or OP1) does not *contain* a conversion - but it *requires* conversion to FINAL_TYPE. */ - - if ((TYPE_PRECISION (TREE_TYPE (op0)) - == TYPE_PRECISION (TREE_TYPE (arg0))) - && TREE_TYPE (op0) != final_type) - unsigned0 = TYPE_UNSIGNED (TREE_TYPE (op0)); - if ((TYPE_PRECISION (TREE_TYPE (op1)) - == TYPE_PRECISION (TREE_TYPE (arg1))) - && TREE_TYPE (op1) != final_type) - unsigned1 = TYPE_UNSIGNED (TREE_TYPE (op1)); - - /* Now UNSIGNED0 is 1 if ARG0 zero-extends to FINAL_TYPE. */ - - /* For bitwise operations, signedness of nominal type - does not matter. Consider only how operands were extended. */ - if (shorten == -1) - uns = unsigned0; - - /* Note that in all three cases below we refrain from optimizing - an unsigned operation on sign-extended args. - That would not be valid. */ - - /* Both args variable: if both extended in same way - from same width, do it in that width. - Do it unsigned if args were zero-extended. */ - if ((TYPE_PRECISION (TREE_TYPE (arg0)) - < TYPE_PRECISION (result_type)) - && (TYPE_PRECISION (TREE_TYPE (arg1)) - == TYPE_PRECISION (TREE_TYPE (arg0))) - && unsigned0 == unsigned1 - && (unsigned0 || !uns)) - result_type - = c_common_signed_or_unsigned_type - (unsigned0, common_type (TREE_TYPE (arg0), TREE_TYPE (arg1))); - else if (TREE_CODE (arg0) == INTEGER_CST - && (unsigned1 || !uns) - && (TYPE_PRECISION (TREE_TYPE (arg1)) - < TYPE_PRECISION (result_type)) - && (type - = c_common_signed_or_unsigned_type (unsigned1, - TREE_TYPE (arg1))) - && !POINTER_TYPE_P (type) - && int_fits_type_p (arg0, type)) - result_type = type; - else if (TREE_CODE (arg1) == INTEGER_CST - && (unsigned0 || !uns) - && (TYPE_PRECISION (TREE_TYPE (arg0)) - < TYPE_PRECISION (result_type)) - && (type - = c_common_signed_or_unsigned_type (unsigned0, - TREE_TYPE (arg0))) - && !POINTER_TYPE_P (type) - && int_fits_type_p (arg1, type)) - result_type = type; + result_type = shorten_binary_op (result_type, op0, op1, + shorten == -1); } /* Shifts can be shortened if shifting right. */ diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index e05b0cb8d94..0e236b1ef3b 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,9 @@ +2008-07-30 Manuel Lopez-Ibanez + + PR 34389 + * typeck.c (build_binary_op): Encapsulate code into + shorten_binary_op. + 2008-07-29 Jakub Jelinek PR c++/36852 diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index fcf52dc9db4..ba1d0286079 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -3810,61 +3810,9 @@ cp_build_binary_op (enum tree_code code, tree orig_op0, tree orig_op1, if (shorten && none_complex) { - int unsigned0, unsigned1; - tree arg0 = get_narrower (op0, &unsigned0); - tree arg1 = get_narrower (op1, &unsigned1); - /* UNS is 1 if the operation to be done is an unsigned one. */ - int uns = TYPE_UNSIGNED (result_type); - tree type; - final_type = result_type; - - /* Handle the case that OP0 does not *contain* a conversion - but it *requires* conversion to FINAL_TYPE. */ - - if (op0 == arg0 && TREE_TYPE (op0) != final_type) - unsigned0 = TYPE_UNSIGNED (TREE_TYPE (op0)); - if (op1 == arg1 && TREE_TYPE (op1) != final_type) - unsigned1 = TYPE_UNSIGNED (TREE_TYPE (op1)); - - /* Now UNSIGNED0 is 1 if ARG0 zero-extends to FINAL_TYPE. */ - - /* For bitwise operations, signedness of nominal type - does not matter. Consider only how operands were extended. */ - if (shorten == -1) - uns = unsigned0; - - /* Note that in all three cases below we refrain from optimizing - an unsigned operation on sign-extended args. - That would not be valid. */ - - /* Both args variable: if both extended in same way - from same width, do it in that width. - Do it unsigned if args were zero-extended. */ - if ((TYPE_PRECISION (TREE_TYPE (arg0)) - < TYPE_PRECISION (result_type)) - && (TYPE_PRECISION (TREE_TYPE (arg1)) - == TYPE_PRECISION (TREE_TYPE (arg0))) - && unsigned0 == unsigned1 - && (unsigned0 || !uns)) - result_type = c_common_signed_or_unsigned_type - (unsigned0, common_type (TREE_TYPE (arg0), TREE_TYPE (arg1))); - else if (TREE_CODE (arg0) == INTEGER_CST - && (unsigned1 || !uns) - && (TYPE_PRECISION (TREE_TYPE (arg1)) - < TYPE_PRECISION (result_type)) - && (type = c_common_signed_or_unsigned_type - (unsigned1, TREE_TYPE (arg1)), - int_fits_type_p (arg0, type))) - result_type = type; - else if (TREE_CODE (arg1) == INTEGER_CST - && (unsigned0 || !uns) - && (TYPE_PRECISION (TREE_TYPE (arg0)) - < TYPE_PRECISION (result_type)) - && (type = c_common_signed_or_unsigned_type - (unsigned0, TREE_TYPE (arg0)), - int_fits_type_p (arg1, type))) - result_type = type; + result_type = shorten_binary_op (result_type, op0, op1, + shorten == -1); } /* Comparison operations are shortened too but differently. diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 93b95f105f9..46820ff4535 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2008-07-30 Manuel Lopez-Ibanez + + PR 34389 + * gcc.dg/Wconversion-pr34389.c: New. + * g++.dg/warn/Wconversion-pr34389.C: New. + 2008-07-29 Steve Ellcey * gcc.dg/pr32370.c: Force 64 bits on IA64. diff --git a/gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C b/gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C new file mode 100644 index 00000000000..f3cd3105386 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C @@ -0,0 +1,53 @@ +/* PR 34389 */ +/* { dg-do compile } */ +/* { dg-options "-Wconversion -Wsign-conversion" } */ + +short mask1(short x) +{ + short y = 0x7fff; + return x & y; /* { dg-bogus "conversion" "conversion" { xfail *-*-* } 8 } */ +} + +short mask2(short ssx) +{ + short ssy; + short ssz; + + ssz = ((int)ssx) & 0x7fff; + ssy = ((int)ssx) | 0x7fff; + ssz = ((int)ssx) ^ 0x7fff; + return ssx & 0x7fff; +} + +short mask3(int si, unsigned int ui) +{ + short ss; + unsigned short us; + + ss = si & 0x7fff; + ss = si & 0xAAAA; /* { dg-warning "conversion" } */ + ss = ui & 0x7fff; + ss = ui & 0xAAAA; /* { dg-warning "conversion" } */ + + us = si & 0x7fff; + us = si & 0xAAAA; /* { dg-warning "conversion" } */ + us = ui & 0x7fff; + us = ui & 0xAAAA; /* { dg-warning "conversion" } */ + + return ss; +} + +short mask4(int x, int y) +{ + return x & y; /* { dg-warning "conversion" } */ +} + +short mask5(int x) +{ + return x & -1; /* { dg-warning "conversion" } */ +} + +short mask6(short x) +{ + return x & -1; +} diff --git a/gcc/testsuite/gcc.dg/Wconversion-pr34389.c b/gcc/testsuite/gcc.dg/Wconversion-pr34389.c new file mode 100644 index 00000000000..45cdf19ef71 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wconversion-pr34389.c @@ -0,0 +1,53 @@ +/* PR 34389 */ +/* { dg-do compile } */ +/* { dg-options "-Wconversion -Wsign-conversion" } */ + +short mask1(short x) +{ + short y = 0x7fff; + return x & y; +} + +short mask2(short ssx) +{ + short ssy; + short ssz; + + ssz = ((int)ssx) & 0x7fff; + ssy = ((int)ssx) | 0x7fff; + ssz = ((int)ssx) ^ 0x7fff; + return ssx & 0x7fff; +} + +short mask3(int si, unsigned int ui) +{ + short ss; + unsigned short us; + + ss = si & 0x7fff; + ss = si & 0xAAAA; /* { dg-warning "conversion" } */ + ss = ui & 0x7fff; + ss = ui & 0xAAAA; /* { dg-warning "conversion" } */ + + us = si & 0x7fff; + us = si & 0xAAAA; /* { dg-warning "conversion" } */ + us = ui & 0x7fff; + us = ui & 0xAAAA; /* { dg-warning "conversion" } */ + + return ss; +} + +short mask4(int x, int y) +{ + return x & y; /* { dg-warning "conversion" } */ +} + +short mask5(int x) +{ + return x & -1; /* { dg-warning "conversion" } */ +} + +short mask6(short x) +{ + return x & -1; +} -- 2.30.2