Ping.
On Tue, Jul 03, 2018 at 09:35:24AM -0400, Marek Polacek wrote:
> This PR complains about bogus -Wsign-conversion warning even with an
> explicit static_cast. It started with this hunk from the delayed folding
> merge:
>
> @@ -5028,20 +5022,12 @@ cp_build_binary_op (location_t location,
>
> if (short_compare)
> {
> - /* Don't write &op0, etc., because that would prevent op0
> - from being kept in a register.
> - Instead, make copies of the our local variables and
> - pass the copies by reference, then copy them back afterward. */
> - tree xop0 = op0, xop1 = op1, xresult_type = result_type;
> + /* We call shorten_compare only for diagnostic-reason. */
> + tree xop0 = fold_simple (op0), xop1 = fold_simple (op1),
> + xresult_type = result_type;
> enum tree_code xresultcode = resultcode;
> - tree val
> - = shorten_compare (location, &xop0, &xop1, &xresult_type,
> + shorten_compare (location, &xop0, &xop1, &xresult_type,
> &xresultcode);
> - if (val != 0)
> - return cp_convert (boolean_type_node, val, complain);
> - op0 = xop0, op1 = xop1;
> - converted = 1;
> - resultcode = xresultcode;
> }
>
> if ((short_compare || code == MIN_EXPR || code == MAX_EXPR)
>
> which means that converted is now unset so we go to
>
> 5350 if (! converted)
> 5351 {
> 5352 if (TREE_TYPE (op0) != result_type)
> 5353 op0 = cp_convert_and_check (result_type, op0, complain);
> 5354 if (TREE_TYPE (op1) != result_type)
> 5355 op1 = cp_convert_and_check (result_type, op1, complain);
>
> and cp_convert_and_check gives those warning. The direct comparison
> of types instead of same_type_p means we can try to convert same types,
> but it still wouldn't fix this PR. What we should probably do is to
> simply disable -Wsign-conversion conversion for comparison, because
> -Wsign-compare will warn for those. With this patch, the C++ FE will
> follow what the C FE and clang++ do.
>
> Also fix some formatting that's been bothering me, while at it.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk/8?
>
> 2018-07-03 Marek Polacek <pola...@redhat.com>
>
> PR c++/86190 - bogus -Wsign-conversion warning
> * typeck.c (cp_build_binary_op): Fix formatting. Add a warning
> sentinel.
>
> * g++.dg/warn/Wsign-conversion-3.C: New test.
> * g++.dg/warn/Wsign-conversion-4.C: New test.
>
> diff --git gcc/cp/typeck.c gcc/cp/typeck.c
> index 3a4f1cdf479..cfd1dd8b150 100644
> --- gcc/cp/typeck.c
> +++ gcc/cp/typeck.c
> @@ -5311,12 +5311,13 @@ cp_build_binary_op (location_t location,
>
> if (short_compare)
> {
> - /* We call shorten_compare only for diagnostic-reason. */
> - tree xop0 = fold_simple (op0), xop1 = fold_simple (op1),
> - xresult_type = result_type;
> + /* We call shorten_compare only for diagnostics. */
> + tree xop0 = fold_simple (op0);
> + tree xop1 = fold_simple (op1);
> + tree xresult_type = result_type;
> enum tree_code xresultcode = resultcode;
> shorten_compare (location, &xop0, &xop1, &xresult_type,
> - &xresultcode);
> + &xresultcode);
> }
>
> if ((short_compare || code == MIN_EXPR || code == MAX_EXPR)
> @@ -5349,6 +5350,7 @@ cp_build_binary_op (location_t location,
> otherwise, it will be given type RESULT_TYPE. */
> if (! converted)
> {
> + warning_sentinel w (warn_sign_conversion, short_compare);
> if (TREE_TYPE (op0) != result_type)
> op0 = cp_convert_and_check (result_type, op0, complain);
> if (TREE_TYPE (op1) != result_type)
> diff --git gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C
> gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C
> index e69de29bb2d..2c3fef31475 100644
> --- gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C
> +++ gcc/testsuite/g++.dg/warn/Wsign-conversion-3.C
> @@ -0,0 +1,13 @@
> +// PR c++/86190
> +// { dg-options "-Wsign-conversion -Wsign-compare" }
> +
> +typedef unsigned long sz_t;
> +sz_t s();
> +bool f(int i) { return s() < (unsigned long) i; }
> +bool f2(int i) { return s() < static_cast<unsigned long>(i); }
> +bool f3(int i) { return s() < i; } // { dg-warning "comparison of integer
> expressions of different signedness" }
> +bool f4(int i) { return s() < (long) i; } // { dg-warning "comparison of
> integer expressions of different signedness" }
> +bool f5(short int i) { return s() < (int) i; } // { dg-warning "comparison
> of integer expressions of different signedness" }
> +bool f6(signed char i) { return s() < (int) i; } // { dg-warning "comparison
> of integer expressions of different signedness" }
> +bool f7(unsigned char i) { return s() < i; }
> +bool f8(signed char i) { return s() < i; } // { dg-warning "comparison of
> integer expressions of different signedness" }
> diff --git gcc/testsuite/g++.dg/warn/Wsign-conversion-4.C
> gcc/testsuite/g++.dg/warn/Wsign-conversion-4.C
> index e69de29bb2d..40814b95587 100644
> --- gcc/testsuite/g++.dg/warn/Wsign-conversion-4.C
> +++ gcc/testsuite/g++.dg/warn/Wsign-conversion-4.C
> @@ -0,0 +1,14 @@
> +// PR c++/86190
> +// { dg-options "-Wsign-conversion -Wsign-compare" }
> +
> +typedef unsigned long size_t;
> +
> +struct vector {
> + typedef size_t size_type;
> + size_type size();
> +};
> +
> +bool func(vector vec, int var)
> +{
> + return vec.size() < static_cast<size_t>(var); // { dg-bogus "may change" }
> +}
Marek
