From 2d12797c692346ff8d9ca935835a3e0b659ab4b8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Manuel=20L=C3=B3pez-Ib=C3=A1=C3=B1ez?= Date: Wed, 6 Aug 2008 16:17:41 +0000 Subject: [PATCH] re PR c++/8715 ('~' operator for unsigned char and conversion to bool) 2008-08-06 Manuel Lopez-Ibanez PR 8715 * c-common.c (warn_for_sign_compare): New. Handle separately the case that 'constant' is zero. * c-typeck.c (build_binary_op): Move code to c-common.c cp/ * typeck.c (cp_build_binary_op): Move code to c-common.c. testsuite/ * gcc.dg/pr8715.c: New. * g++.dg/warn/pr8715.C: New. From-SVN: r138814 --- gcc/ChangeLog | 7 ++ gcc/c-common.c | 141 +++++++++++++++++++++++++++++ gcc/c-common.h | 4 + gcc/c-typeck.c | 122 +------------------------ gcc/cp/ChangeLog | 5 + gcc/cp/typeck.c | 112 +---------------------- gcc/testsuite/ChangeLog | 6 ++ gcc/testsuite/g++.dg/warn/pr8715.C | 11 +++ gcc/testsuite/gcc.dg/pr8715.c | 13 +++ 9 files changed, 195 insertions(+), 226 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/pr8715.C create mode 100644 gcc/testsuite/gcc.dg/pr8715.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index a819795d7088..47226532a561 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,10 @@ +2008-08-06 Manuel Lopez-Ibanez + + PR 8715 + * c-common.c (warn_for_sign_compare): New. Handle separately the + case that 'constant' is zero. + * c-typeck.c (build_binary_op): Move code to c-common.c + 2008-08-06 Kaveh R. Ghazi * config/alpha/alpha.c (alpha_preferred_reload_class, diff --git a/gcc/c-common.c b/gcc/c-common.c index dac29ea2c7e8..d0ff04be79d2 100644 --- a/gcc/c-common.c +++ b/gcc/c-common.c @@ -8202,4 +8202,145 @@ warn_for_div_by_zero (tree divisor) warning (OPT_Wdiv_by_zero, "division by zero"); } +/* Subroutine of build_binary_op. Give warnings for comparisons + between signed and unsigned quantities that may fail. Do the + checking based on the original operand trees ORIG_OP0 and ORIG_OP1, + so that casts will be considered, but default promotions won't + be. + + The arguments of this function map directly to local variables + of build_binary_op. */ + +void +warn_for_sign_compare (tree orig_op0, tree orig_op1, + tree op0, tree op1, + tree result_type, enum tree_code resultcode) +{ + int op0_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op0)); + int op1_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op1)); + int unsignedp0, unsignedp1; + + /* In C++, check for comparison of different enum types. */ + if (c_dialect_cxx() + && TREE_CODE (TREE_TYPE (orig_op0)) == ENUMERAL_TYPE + && TREE_CODE (TREE_TYPE (orig_op1)) == ENUMERAL_TYPE + && TYPE_MAIN_VARIANT (TREE_TYPE (orig_op0)) + != TYPE_MAIN_VARIANT (TREE_TYPE (orig_op1))) + { + warning (OPT_Wsign_compare, "comparison between types %qT and %qT", + TREE_TYPE (orig_op0), TREE_TYPE (orig_op1)); + } + + /* Do not warn if the comparison is being done in a signed type, + since the signed type will only be chosen if it can represent + all the values of the unsigned type. */ + if (!TYPE_UNSIGNED (result_type)) + /* OK */; + /* Do not warn if both operands are unsigned. */ + else if (op0_signed == op1_signed) + /* OK */; + else + { + tree sop, uop; + bool ovf; + + if (op0_signed) + sop = orig_op0, uop = orig_op1; + else + sop = orig_op1, uop = orig_op0; + + STRIP_TYPE_NOPS (sop); + STRIP_TYPE_NOPS (uop); + + /* Do not warn if the signed quantity is an unsuffixed integer + literal (or some static constant expression involving such + literals or a conditional expression involving such literals) + and it is non-negative. */ + if (tree_expr_nonnegative_warnv_p (sop, &ovf)) + /* 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, c_common_signed_type (result_type))) + /* OK */; + /* In C, 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 (!c_dialect_cxx() && TREE_CODE (uop) == INTEGER_CST + && TREE_CODE (TREE_TYPE (uop)) == ENUMERAL_TYPE + && int_fits_type_p (TYPE_MAX_VALUE (TREE_TYPE (uop)), + c_common_signed_type (result_type))) + /* OK */; + else + warning (OPT_Wsign_compare, + "comparison between signed and unsigned integer expressions"); + } + + /* Warn if two unsigned values are being compared in a size larger + than their original size, and one (and only one) is the result of + a `~' operator. This comparison will always fail. + + Also warn if one operand is a constant, and the constant does not + have all bits set that are set in the ~ operand when it is + extended. */ + + op0 = get_narrower (op0, &unsignedp0); + op1 = get_narrower (op1, &unsignedp1); + + if ((TREE_CODE (op0) == BIT_NOT_EXPR) + ^ (TREE_CODE (op1) == BIT_NOT_EXPR)) + { + if (TREE_CODE (op0) == BIT_NOT_EXPR) + op0 = get_narrower (TREE_OPERAND (op0, 0), &unsignedp0); + if (TREE_CODE (op1) == BIT_NOT_EXPR) + op1 = get_narrower (TREE_OPERAND (op1, 0), &unsignedp1); + + if (host_integerp (op0, 0) || host_integerp (op1, 0)) + { + tree primop; + HOST_WIDE_INT constant, mask; + int unsignedp; + unsigned int bits; + + if (host_integerp (op0, 0)) + { + primop = op1; + unsignedp = unsignedp1; + constant = tree_low_cst (op0, 0); + } + else + { + primop = op0; + unsignedp = unsignedp0; + constant = tree_low_cst (op1, 0); + } + + bits = TYPE_PRECISION (TREE_TYPE (primop)); + if (bits < TYPE_PRECISION (result_type) + && bits < HOST_BITS_PER_LONG && unsignedp) + { + mask = (~ (HOST_WIDE_INT) 0) << bits; + if ((mask & constant) != mask) + { + if (constant == 0) + warning (OPT_Wsign_compare, + "promoted ~unsigned is always non-zero"); + else + warning (OPT_Wsign_compare, + "comparison of promoted ~unsigned with constant"); + } + } + } + else if (unsignedp0 && unsignedp1 + && (TYPE_PRECISION (TREE_TYPE (op0)) + < TYPE_PRECISION (result_type)) + && (TYPE_PRECISION (TREE_TYPE (op1)) + < TYPE_PRECISION (result_type))) + warning (OPT_Wsign_compare, + "comparison of promoted ~unsigned with unsigned"); + } +} + #include "gt-c-common.h" diff --git a/gcc/c-common.h b/gcc/c-common.h index f600751f0c08..0f2a359c16f0 100644 --- a/gcc/c-common.h +++ b/gcc/c-common.h @@ -928,6 +928,10 @@ extern void warn_about_parentheses (enum tree_code, enum tree_code, enum tree_code); extern void warn_for_unused_label (tree label); extern void warn_for_div_by_zero (tree divisor); +extern void warn_for_sign_compare (tree orig_op0, tree orig_op1, + tree op0, tree op1, + tree result_type, + enum tree_code resultcode); /* In c-gimplify.c */ extern void c_genericize (tree); diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c index 4756e256f387..bacff90be02a 100644 --- a/gcc/c-typeck.c +++ b/gcc/c-typeck.c @@ -8372,124 +8372,10 @@ build_binary_op (enum tree_code code, tree orig_op0, tree orig_op1, converted = 1; resultcode = xresultcode; - if (warn_sign_compare && skip_evaluation == 0) - { - int op0_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op0)); - int op1_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op1)); - int unsignedp0, unsignedp1; - tree primop0 = get_narrower (op0, &unsignedp0); - tree primop1 = get_narrower (op1, &unsignedp1); - - xop0 = orig_op0; - xop1 = orig_op1; - STRIP_TYPE_NOPS (xop0); - STRIP_TYPE_NOPS (xop1); - - /* Give warnings for comparisons between signed and unsigned - quantities that may fail. - - Do the checking based on the original operand trees, so that - casts will be considered, but default promotions won't be. - - Do not warn if the comparison is being done in a signed type, - since the signed type will only be chosen if it can represent - all the values of the unsigned type. */ - if (!TYPE_UNSIGNED (result_type)) - /* OK */; - /* Do not warn if both operands are the same signedness. */ - else if (op0_signed == op1_signed) - /* OK */; - else - { - tree sop, uop; - bool ovf; - - 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 or a - conditional expression involving such literals) - and it is non-negative. */ - if (tree_expr_nonnegative_warnv_p (sop, &ovf)) - /* 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, c_common_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)), - c_common_signed_type (result_type))) - /* OK */; - else - warning (OPT_Wsign_compare, "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 - result of a `~' operator. This comparison will always fail. - - Also warn if one operand is a constant, and the constant - does not have all bits set that are set in the ~ operand - when it is extended. */ - - if ((TREE_CODE (primop0) == BIT_NOT_EXPR) - != (TREE_CODE (primop1) == BIT_NOT_EXPR)) - { - if (TREE_CODE (primop0) == BIT_NOT_EXPR) - primop0 = get_narrower (TREE_OPERAND (primop0, 0), - &unsignedp0); - else - primop1 = get_narrower (TREE_OPERAND (primop1, 0), - &unsignedp1); - - if (host_integerp (primop0, 0) || host_integerp (primop1, 0)) - { - tree primop; - HOST_WIDE_INT constant, mask; - int unsignedp, bits; - - if (host_integerp (primop0, 0)) - { - primop = primop1; - unsignedp = unsignedp1; - constant = tree_low_cst (primop0, 0); - } - else - { - primop = primop0; - unsignedp = unsignedp0; - constant = tree_low_cst (primop1, 0); - } - - bits = TYPE_PRECISION (TREE_TYPE (primop)); - if (bits < TYPE_PRECISION (result_type) - && bits < HOST_BITS_PER_WIDE_INT && unsignedp) - { - mask = (~(HOST_WIDE_INT) 0) << bits; - if ((mask & constant) != mask) - warning (OPT_Wsign_compare, "comparison of promoted ~unsigned with constant"); - } - } - else if (unsignedp0 && unsignedp1 - && (TYPE_PRECISION (TREE_TYPE (primop0)) - < TYPE_PRECISION (result_type)) - && (TYPE_PRECISION (TREE_TYPE (primop1)) - < TYPE_PRECISION (result_type))) - warning (OPT_Wsign_compare, "comparison of promoted ~unsigned with unsigned"); - } + if (warn_sign_compare && !skip_evaluation) + { + warn_for_sign_compare (orig_op0, orig_op1, op0, op1, + result_type, resultcode); } } } diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 8ece21d96f96..c55e4b5c3066 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,8 @@ +2008-08-06 Manuel Lopez-Ibanez + + PR 8715 + * typeck.c (cp_build_binary_op): Move code to c-common.c. + 2008-08-05 Jason Merrill PR c++/37016 diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index fd3dba9345c8..792a77cc1a85 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -3839,115 +3839,11 @@ cp_build_binary_op (enum tree_code code, tree orig_op0, tree orig_op1, && warn_sign_compare /* Do not warn until the template is instantiated; we cannot bound the ranges of the arguments until that point. */ - && !processing_template_decl) + && !processing_template_decl + && (complain & tf_warning)) { - int op0_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op0)); - int op1_signed = !TYPE_UNSIGNED (TREE_TYPE (orig_op1)); - - int unsignedp0, unsignedp1; - tree primop0 = get_narrower (op0, &unsignedp0); - tree primop1 = get_narrower (op1, &unsignedp1); - - /* Check for comparison of different enum types. */ - if (TREE_CODE (TREE_TYPE (orig_op0)) == ENUMERAL_TYPE - && TREE_CODE (TREE_TYPE (orig_op1)) == ENUMERAL_TYPE - && TYPE_MAIN_VARIANT (TREE_TYPE (orig_op0)) - != TYPE_MAIN_VARIANT (TREE_TYPE (orig_op1)) - && (complain & tf_warning)) - { - warning (OPT_Wsign_compare, "comparison between types %q#T and %q#T", - TREE_TYPE (orig_op0), TREE_TYPE (orig_op1)); - } - - /* Give warnings for comparisons between signed and unsigned - quantities that may fail. */ - /* Do the checking based on the original operand trees, so that - casts will be considered, but default promotions won't be. */ - - /* Do not warn if the comparison is being done in a signed type, - since the signed type will only be chosen if it can represent - all the values of the unsigned type. */ - if (!TYPE_UNSIGNED (result_type)) - /* OK */; - /* Do not warn if both operands are unsigned. */ - 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 or a conditional expression - involving such literals) and it is non-negative. */ - else if ((op0_signed && tree_expr_nonnegative_p (orig_op0)) - || (op1_signed && tree_expr_nonnegative_p (orig_op1))) - /* 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 (orig_op1) == INTEGER_CST - && int_fits_type_p (orig_op1, c_common_signed_type - (result_type))) - || (op1_signed && TREE_CODE (orig_op0) == INTEGER_CST - && int_fits_type_p (orig_op0, c_common_signed_type - (result_type))))) - /* OK */; - else if (complain & tf_warning) - warning (OPT_Wsign_compare, - "comparison between signed and unsigned integer expressions"); - - /* Warn if two unsigned values are being compared in a size - larger than their original size, and one (and only one) is the - result of a `~' operator. This comparison will always fail. - - Also warn if one operand is a constant, and the constant does not - have all bits set that are set in the ~ operand when it is - extended. */ - - if ((TREE_CODE (primop0) == BIT_NOT_EXPR) - ^ (TREE_CODE (primop1) == BIT_NOT_EXPR)) - { - if (TREE_CODE (primop0) == BIT_NOT_EXPR) - primop0 = get_narrower (TREE_OPERAND (op0, 0), &unsignedp0); - if (TREE_CODE (primop1) == BIT_NOT_EXPR) - primop1 = get_narrower (TREE_OPERAND (op1, 0), &unsignedp1); - - if (host_integerp (primop0, 0) || host_integerp (primop1, 0)) - { - tree primop; - HOST_WIDE_INT constant, mask; - int unsignedp; - unsigned int bits; - - if (host_integerp (primop0, 0)) - { - primop = primop1; - unsignedp = unsignedp1; - constant = tree_low_cst (primop0, 0); - } - else - { - primop = primop0; - unsignedp = unsignedp0; - constant = tree_low_cst (primop1, 0); - } - - bits = TYPE_PRECISION (TREE_TYPE (primop)); - if (bits < TYPE_PRECISION (result_type) - && bits < HOST_BITS_PER_LONG && unsignedp) - { - mask = (~ (HOST_WIDE_INT) 0) << bits; - if ((mask & constant) != mask - && (complain & tf_warning)) - warning (OPT_Wsign_compare, "comparison of promoted ~unsigned with constant"); - } - } - else if (unsignedp0 && unsignedp1 - && (TYPE_PRECISION (TREE_TYPE (primop0)) - < TYPE_PRECISION (result_type)) - && (TYPE_PRECISION (TREE_TYPE (primop1)) - < TYPE_PRECISION (result_type)) - && (complain & tf_warning)) - warning (OPT_Wsign_compare, "comparison of promoted ~unsigned with unsigned"); - } + warn_for_sign_compare (orig_op0, orig_op1, op0, op1, + result_type, resultcode); } } diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index a8b0b14924d9..ad00fee706c2 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2008-08-06 Manuel Lopez-Ibanez + + PR 8715 + * gcc.dg/pr8715.c: New. + * g++.dg/warn/pr8715.C: New. + 2008-08-06 Marc Gauthier * lib/target-supports.exp (check_profiling_available): Match more diff --git a/gcc/testsuite/g++.dg/warn/pr8715.C b/gcc/testsuite/g++.dg/warn/pr8715.C new file mode 100644 index 000000000000..330c148bb59d --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/pr8715.C @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-Wsign-compare" } */ + +int foo() +{ + unsigned char b = '1'; + + bool x = ~b; /* { dg-warning "promoted ~unsigned is always non-zero" } */ + + return 0; +} diff --git a/gcc/testsuite/gcc.dg/pr8715.c b/gcc/testsuite/gcc.dg/pr8715.c new file mode 100644 index 000000000000..e45e77c09f4c --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr8715.c @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-Wsign-compare -std=c99" } */ + +#include + +int foo() +{ + unsigned char b = '1'; + + bool x = ~b; /* { dg-warning "promoted ~unsigned is always non-zero" } */ + + return 0; +} -- 2.47.2