This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PR c/c++ 34389 -Wconversion produces wrong warning
Updated and regression tested after tuples merge.
OK?
2008/7/25 Manuel López-Ibáñez <lopezibanez@gmail.com>:
> [I have sent this a couple of times already. Is there any issue with it?]
>
> This patch never got reviewed. I have updated it against a recent
> revision (again).
>
> To avoid rehearsing the same points as before:
>
> * There is an XFAIL because common_types does completely different
> things in C and C++. I think that is a latent bug exposed by my patch.
>
> * The patch does not remove shorten_binary_op even though that
> optimisation is supposedly done by fold. I need the function for
> conversion_warnings and there was some concern (by Jakub) whether the
> optimisation is actually done by fold. My impression is that the
> current code does nothing at all, since fold precisely does the
> opposite transformation. In any case, that could be done in a
> follow-up patch. I would like to fix this bug.
>
> Let me know your opinions and suggestions.
>
> Bootstrapped and regression tested in x86_64-unknown-linux-gnu.
>
> Cheers,
>
> Manuel.
>
> 2008-06-10 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 the new function.
> testsuite/
> * gcc.dg/Wconversion-pr34389.c: New.
> * g++.dg/warn/Wconversion-pr34389.C: New.
>
Index: gcc/testsuite/gcc.dg/Wconversion-pr34389.c
===================================================================
--- gcc/testsuite/gcc.dg/Wconversion-pr34389.c (revision 0)
+++ gcc/testsuite/gcc.dg/Wconversion-pr34389.c (revision 0)
@@ -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;
+}
Index: gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C (revision 0)
+++ gcc/testsuite/g++.dg/warn/Wconversion-pr34389.C (revision 0)
@@ -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;
+}
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c (revision 138207)
+++ gcc/cp/typeck.c (working copy)
@@ -3808,65 +3808,13 @@ cp_build_binary_op (enum tree_code code,
E.g., (short)-1 | (unsigned short)-1 is (int)-1
but calculated in (unsigned short) it would be (unsigned short)-1. */
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.
They identify themselves by setting short_compare = 1. */
Index: gcc/c-typeck.c
===================================================================
--- gcc/c-typeck.c (revision 138207)
+++ gcc/c-typeck.c (working copy)
@@ -8314,97 +8314,13 @@ build_binary_op (enum tree_code code, tr
Eg, (short)-1 | (unsigned short)-1 is (int)-1
but calculated in (unsigned short) it would be (unsigned short)-1. */
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. */
if (short_shift)
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c (revision 138207)
+++ gcc/c-common.c (working copy)
@@ -1445,10 +1445,114 @@ vector_types_convertible_p (const_tree t
}
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. */
static void
conversion_warning (tree type, tree expr)
@@ -1509,65 +1613,96 @@ 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 (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);
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
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h (revision 138207)
+++ gcc/c-common.h (working copy)
@@ -741,10 +741,13 @@ extern bool c_determine_visibility (tree
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. */
extern tree shorten_compare (tree *, tree *, tree *, enum tree_code *);