If compiled with gcc -c -Wconversion the following code produces the warning: test.c:7: warning: conversion to ‘short int’ from ‘int’ may alter its value but if -DNO_WARNING is turned on it doesn't even though it should be same code. cat > test.c <<EOF short mask(short x) { #if NO_WARNING short y = 0x7fff; return x&y; #else return x & (short)0x7fff; #endif } EOF On a side node, I think it would be better to also change the warning to : "conversion from 'int' to 'short int' may alter its value", which i find easier to read.
Confirmed. When the expression "x & (short)0x7fff" reaches the warning code, it sees "(int)x & 0x7fff". So it rightfully warns that an "int" expression is converted to "short". This comes from conversions performed in build_binary_op. Not sure what the appropriate fix would be...
I think this is related to PR 32643.
(In reply to comment #2) > I think this is related to PR 32643. > How is it related? There is no overflow here, is there? It is related in the sense that build_binary_op is playing tricks with bitwise AND ?
@Andrew, I would like to fix this before GCC 4.3 is released but I just don't know how. Any hints?
I don't think this will be fixed before GCC 4.3 is released, which is a shame. But if anyone has a suggestion on how to fix this, I will try to submit a patch ASAP.
Some more findings: build_binary_op receives two parameters of type int: '(int)x' and '0x7fff' then it performs the shorten magic that seems right and produces: <bit_and_expr type <integer_type short int arg 0 <parm_decl x type <integer_type short int> arg 1 <integer_cst type <integer_type short int> constant invariant 32767>> which looks good. But just before returning, it converts that to 'int', what results in: <bit_and_expr type <integer_type int arg 0 <nop_expr type <integer_type int> arg 0 <parm_decl x type <integer_type short int>> arg 1 <integer_cst type <integer_type int>> which causes problems later. If instead it returned: <nop_expr type <integer_type int> <bit_and_expr type <integer_type short int arg 0 <parm_decl x type <integer_type short int> arg 1 <integer_cst type <integer_type short int> constant invariant>> No warning would be generated! Why bother with the shorten magic if convert is going to override it? Where it is overriding it? The current code converts the constant back and forth between short int and int at least 3 times. Any ideas on how to stop this madness?
So fold_unary is transforming (T)(x & c) into (T)x & (T)c. Which is exactly the opposite transformation that build_binary_op just performed! Weird... So I see two options: * Either teach fold_unary to avoid this particular transformation in this particular case. * Hack conversion_warning to realize that (short)((int)x & c) is the same as (short)(x & c) if x is signed int and c fits in a signed int. Which will be undoing the transformation once more (fourth? fifth time?). And I am not even sure how to do this.
This patch fixes this bug but it is an ugly hack. Perhaps such an ugly hack is the only thing we can do at the moment. Once bootstrapping + testing finishes, I will submit. Index: gcc/c-common.c =================================================================== --- gcc/c-common.c (revision 132310) +++ gcc/c-common.c (working copy) @@ -1270,65 +1270,98 @@ conversion_warning (tree type, tree expr "conversion to %qT alters %qT constant value", type, TREE_TYPE (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 (TREE_OPERAND (expr, 1)) == INTEGER_CST) + { + int unsigned0, unsigned1; + tree arg0, arg1; + tree tmp_type; + tree op0, op1; + + op0 = convert (expr_type, TREE_OPERAND (expr, 0)); + op1 = convert (expr_type, TREE_OPERAND (expr, 1)); + + arg0 = get_narrower (op0, &unsigned0); + arg1 = get_narrower (op1, &unsigned1); + + /* Handle the case that OP0 (or OP1) does not *contain* a conversion + but it *requires* conversion to TYPE. */ + if ((TYPE_PRECISION (TREE_TYPE (op0)) + == TYPE_PRECISION (TREE_TYPE (arg0))) + && TREE_TYPE (op0) != expr_type) + unsigned0 = TYPE_UNSIGNED (TREE_TYPE (op0)); + + if (TYPE_PRECISION (TREE_TYPE (arg0)) < TYPE_PRECISION (expr_type) + && (tmp_type + = c_common_signed_or_unsigned_type (unsigned0, + TREE_TYPE (arg0))) + && !POINTER_TYPE_P (tmp_type) + && int_fits_type_p (arg1, tmp_type)) + expr_type = tmp_type; + } /* 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); if (!exact_real_truncate (TYPE_MODE (type), &real_low_bound) || !exact_real_truncate (TYPE_MODE (type), &real_high_bound)) give_warning = true; } /* 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); } } /* Produce warnings after a conversion. RESULT is the result of converting EXPR to TYPE. This is a helper function for
Does the patch also fix the warning for conditional expressions? For example: short f(int cond, short x, short y) { return cond ? x : y; }
(In reply to comment #9) > Does the patch also fix the warning for conditional expressions? For example: > > short f(int cond, short x, short y) > { > return cond ? x : y; > } > No, that is a completely different issue.
Subject: Bug 34389 Author: manu Date: Wed Jul 30 08:30:32 2008 New Revision: 138296 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=138296 Log: 2008-07-30 Manuel Lopez-Ibanez <manu@gcc.gnu.org> 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. Added: trunk/gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C trunk/gcc/testsuite/gcc.dg/Wconversion-pr34389.c Modified: trunk/gcc/ChangeLog trunk/gcc/c-common.c trunk/gcc/c-common.h trunk/gcc/c-typeck.c trunk/gcc/cp/ChangeLog trunk/gcc/cp/typeck.c trunk/gcc/testsuite/ChangeLog
This should be mostly fixed except the following testcase in C++: short mask1(short x) { short y = 0x7fff; return x & y; /* { dg-bogus "conversion" "conversion" { xfail *-*-* } 8 } */ } This works in C, so it seems "same_type" does different things in C and in C++. I don't have time to investigate this at the moment so any help is appreciated.
I created PR 37004 to cover the issue with C++ front-end. Apart from that, this bug is FIXED in GCC 4.4.