This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
C++ PATCH for c++/86190, bogus -Wsign-conversion warning
- From: Marek Polacek <polacek at redhat dot com>
- To: Jason Merrill <jason at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 3 Jul 2018 09:35:24 -0400
- Subject: C++ PATCH for c++/86190, bogus -Wsign-conversion warning
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 <polacek@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" }
+}