c-family: Implement -Warray-compare [PR97573]

Message ID 20211001201108.78108-1-polacek@redhat.com
State New
Headers
Series c-family: Implement -Warray-compare [PR97573] |

Commit Message

Marek Polacek Oct. 1, 2021, 8:11 p.m. UTC
  This patch addresses one of my leftovers from GCC 11.  C++20 introduced
[depr.array.comp]: "Equality and relational comparisons between two operands
of array type are deprecated." so this patch adds -Warray-compare.  Since the
code in question is dubious (the comparison doesn't actually compare the array
elements), I've added this warning for C too, and enabled it in all C++ modes.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

	PR c++/97573

gcc/c-family/ChangeLog:

	* c-common.h (do_warn_array_compare): Declare.
	* c-warn.c (do_warn_array_compare): New.
	* c.opt (Warray-compare): New option.

gcc/c/ChangeLog:

	* c-typeck.c (parser_build_binary_op): Call do_warn_array_compare.

gcc/cp/ChangeLog:

	* typeck.c (cp_build_binary_op): Call do_warn_array_compare.

gcc/ChangeLog:

	* doc/invoke.texi: Document -Warray-compare.

gcc/testsuite/ChangeLog:

	* c-c++-common/Warray-compare-1.c: New test.
	* c-c++-common/Warray-compare-2.c: New test.
---
 gcc/c-family/c-common.h                       |  1 +
 gcc/c-family/c-warn.c                         | 32 ++++++++++++++
 gcc/c-family/c.opt                            |  4 ++
 gcc/c/c-typeck.c                              |  9 +++-
 gcc/cp/typeck.c                               | 13 ++++++
 gcc/doc/invoke.texi                           | 20 ++++++++-
 gcc/testsuite/c-c++-common/Warray-compare-1.c | 44 +++++++++++++++++++
 gcc/testsuite/c-c++-common/Warray-compare-2.c | 44 +++++++++++++++++++
 8 files changed, 164 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Warray-compare-1.c
 create mode 100644 gcc/testsuite/c-c++-common/Warray-compare-2.c


base-commit: acf3a21cbc26b39b73c0006300f35ff017ddd6cb
  

Comments

Jakub Jelinek Oct. 1, 2021, 9:15 p.m. UTC | #1
On Fri, Oct 01, 2021 at 04:11:08PM -0400, Marek Polacek via Gcc-patches wrote:
> +  auto_diagnostic_group d;
> +  if (warning_at (location, OPT_Warray_compare,
> +		  "comparison between two arrays%s",
> +		  (c_dialect_cxx () && cxx_dialect >= cxx20)
> +		  ? " is deprecated in C++20" : ""))

Not a review, just a comment.
The above is impossible to translate, translators would translate the
first half and the second one would be in English; but even translating
the second part too would mean one can't reword it in some other language
in different word order.

You can e.g. use
  if (warning_at (location, OPT_Warray_compare,
		  (c_dialect_cxx () && cxx_dialect >= cxx20)
		  ? G_("comparison between two arrays is deprecated in C++20")
		  : G_("comparison between two arrays")))
instead.

	Jakub
  

Patch

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 849cefab882..078730f1e64 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1421,6 +1421,7 @@  extern bool warn_for_restrict (unsigned, tree *, unsigned);
 extern void warn_for_address_or_pointer_of_packed_member (tree, tree);
 extern void warn_parm_array_mismatch (location_t, tree, tree);
 extern void maybe_warn_sizeof_array_div (location_t, tree, tree, tree, tree);
+extern void do_warn_array_compare (location_t, tree_code, tree, tree);
 
 /* Places where an lvalue, or modifiable lvalue, may be required.
    Used to select diagnostic messages in lvalue_error and
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index 84ad6633c96..96aff1c0e5a 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -3726,3 +3726,35 @@  maybe_warn_sizeof_array_div (location_t loc, tree arr, tree arr_type,
 	}
     }
 }
+
+/* Warn about C++20 [depr.array.comp] array comparisons: "Equality
+   and relational comparisons between two operands of array type are
+   deprecated."  We also warn in C and earlier C++ standards.  CODE is
+   the code for this comparison, OP0 and OP1 are the operands.  */
+
+void
+do_warn_array_compare (location_t location, tree_code code, tree op0, tree op1)
+{
+  STRIP_NOPS (op0);
+  STRIP_NOPS (op1);
+  if (TREE_CODE (op0) == ADDR_EXPR)
+    op0 = TREE_OPERAND (op0, 0);
+  if (TREE_CODE (op1) == ADDR_EXPR)
+    op1 = TREE_OPERAND (op1, 0);
+
+  auto_diagnostic_group d;
+  if (warning_at (location, OPT_Warray_compare,
+		  "comparison between two arrays%s",
+		  (c_dialect_cxx () && cxx_dialect >= cxx20)
+		  ? " is deprecated in C++20" : ""))
+    {
+      /* C doesn't allow +arr.  */
+      if (c_dialect_cxx ())
+	inform (location, "use unary %<+%> which decays operands to pointers "
+		"or %<&%D[0] %s &%D[0]%> to compare the addresses",
+		op0, op_symbol_code (code), op1);
+      else
+	inform (location, "use %<&%D[0] %s &%D[0]%> to compare the addresses",
+		op0, op_symbol_code (code), op1);
+    }
+}
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 9c151d19870..06457ac739e 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -350,6 +350,10 @@  Warray-bounds=
 LangEnabledBy(C ObjC C++ LTO ObjC++,Wall,1,0)
 ; in common.opt
 
+Warray-compare
+C ObjC C++ ObjC++ Var(warn_array_compare) Warning LangEnabledBy(C ObjC C++ ObjC++, Wall)
+Warn about comparisons between two operands of array type.
+
 Warray-parameter
 C ObjC C++ ObjC++ Warning Alias(Warray-parameter=, 2, 0)
 Warn about mismatched declarations of array parameters and unsafe accesses to them.
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 33963d7555a..f9eb0e5176f 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3940,7 +3940,14 @@  parser_build_binary_op (location_t location, enum tree_code code,
   else if (TREE_CODE_CLASS (code) == tcc_comparison
 	   && (code1 == STRING_CST || code2 == STRING_CST))
     warning_at (location, OPT_Waddress,
-		"comparison with string literal results in unspecified behavior");
+		"comparison with string literal results in unspecified "
+		"behavior");
+
+  if (warn_array_compare
+      && TREE_CODE_CLASS (code) == tcc_comparison
+      && TREE_CODE (type1) == ARRAY_TYPE
+      && TREE_CODE (type2) == ARRAY_TYPE)
+    do_warn_array_compare (location, code, arg1.value, arg2.value);
 
   if (TREE_OVERFLOW_P (result.value)
       && !TREE_OVERFLOW_P (arg1.value)
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index e880d34dcfe..ab0f9da2552 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -5357,6 +5357,11 @@  cp_build_binary_op (const op_location_t &location,
 	    warning_at (location, OPT_Waddress,
 			"comparison with string literal results in "
 			"unspecified behavior");
+	  else if (warn_array_compare
+		   && TREE_CODE (TREE_TYPE (orig_op0)) == ARRAY_TYPE
+		   && TREE_CODE (TREE_TYPE (orig_op1)) == ARRAY_TYPE)
+	    do_warn_array_compare (location, code, stripped_orig_op0,
+				   stripped_orig_op1);
 	}
 
       build_type = boolean_type_node;
@@ -5629,6 +5634,14 @@  cp_build_binary_op (const op_location_t &location,
 			"comparison with string literal results "
 			"in unspecified behavior");
 	}
+      else if (warn_array_compare
+	       && TREE_CODE (TREE_TYPE (orig_op0)) == ARRAY_TYPE
+	       && TREE_CODE (TREE_TYPE (orig_op1)) == ARRAY_TYPE
+	       && code != SPACESHIP_EXPR
+	       && (complain & tf_warning))
+	do_warn_array_compare (location, code,
+			       tree_strip_any_location_wrapper (orig_op0),
+			       tree_strip_any_location_wrapper (orig_op1));
 
       if (gnu_vector_type_p (type0) && gnu_vector_type_p (type1))
 	{
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d35114c0727..a397afe1caf 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -249,7 +249,8 @@  in the following sections.
 -Wcomma-subscript  -Wconditionally-supported @gol
 -Wno-conversion-null  -Wctad-maybe-unsupported @gol
 -Wctor-dtor-privacy  -Wno-delete-incomplete @gol
--Wdelete-non-virtual-dtor  -Wdeprecated-copy -Wdeprecated-copy-dtor @gol
+-Wdelete-non-virtual-dtor  -Wno-deprecated-array-compare @gol
+-Wdeprecated-copy -Wdeprecated-copy-dtor @gol
 -Wno-deprecated-enum-enum-conversion -Wno-deprecated-enum-float-conversion @gol
 -Weffc++  -Wno-exceptions -Wextra-semi  -Wno-inaccessible-base @gol
 -Wno-inherited-variadic-ctor  -Wno-init-list-lifetime @gol
@@ -323,7 +324,7 @@  Objective-C and Objective-C++ Dialects}.
 -Walloca  -Walloca-larger-than=@var{byte-size} @gol
 -Wno-aggressive-loop-optimizations @gol
 -Warith-conversion @gol
--Warray-bounds  -Warray-bounds=@var{n} @gol
+-Warray-bounds  -Warray-bounds=@var{n}  -Warray-compare @gol
 -Wno-attributes  -Wattribute-alias=@var{n} -Wno-attribute-alias @gol
 -Wno-attribute-warning  -Wbool-compare  -Wbool-operation @gol
 -Wno-builtin-declaration-mismatch @gol
@@ -5562,6 +5563,7 @@  Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
 
 @gccoptlist{-Waddress   @gol
 -Warray-bounds=1 @r{(only with} @option{-O2}@r{)}  @gol
+-Warray-compare @gol
 -Warray-parameter=2 @r{(C and Objective-C only)} @gol
 -Wbool-compare  @gol
 -Wbool-operation  @gol
@@ -7533,6 +7535,20 @@  pointers. This warning level may give a larger number of
 false positives and is deactivated by default.
 @end table
 
+@item -Warray-compare
+@opindex Warray-compare
+@opindex Wno-array-compare
+Warn about equality and relational comparisons between two operands of array
+type.  This comparison was deprecated in C++20.  For example:
+
+@smallexample
+int arr1[5];
+int arr2[5];
+bool same = arr1 == arr2;
+@end smallexample
+
+@option{-Warray-compare} is enabled by @option{-Wall}.
+
 @item -Warray-parameter
 @itemx -Warray-parameter=@var{n}
 @opindex Wno-array-parameter
diff --git a/gcc/testsuite/c-c++-common/Warray-compare-1.c b/gcc/testsuite/c-c++-common/Warray-compare-1.c
new file mode 100644
index 00000000000..922396c0a1a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Warray-compare-1.c
@@ -0,0 +1,44 @@ 
+/* PR c++/97573 */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+#ifndef __cplusplus
+# define bool _Bool
+#endif
+
+int arr1[5];
+int arr2[5];
+int arr3[2][2];
+int arr4[2][2];
+
+bool
+g ()
+{
+  bool b = arr1 == arr2; /* { dg-warning "comparison between two arrays" } */
+  b &= arr1 != arr2; /* { dg-warning "comparison between two arrays" } */
+  b &= arr1 > arr2; /* { dg-warning "comparison between two arrays" } */
+  b &= arr1 >= arr2; /* { dg-warning "comparison between two arrays" } */
+  b &= arr1 < arr2; /* { dg-warning "comparison between two arrays" } */
+  b &= arr1 <= arr2; /* { dg-warning "comparison between two arrays" } */
+#ifdef __cplusplus
+  b &= +arr1 == +arr2;
+  b &= +arr1 != +arr2;
+  b &= +arr1 > +arr2;
+  b &= +arr1 >= +arr2;
+  b &= +arr1 < +arr2;
+  b &= +arr1 <= +arr2;
+#endif
+  b &= &arr1[0] == &arr2[0];
+  b &= &arr1[0] != &arr2[0];
+  b &= &arr1[0] > &arr2[0];
+  b &= &arr1[0] >= &arr2[0];
+  b &= &arr1[0] < &arr2[0];
+  b &= &arr1[0] <= &arr2[0];
+
+  b &= arr3 == arr4; /* { dg-warning "comparison between two arrays" } */
+
+#if defined(__cplusplus) && __cplusplus > 201703L
+  auto cmp = arr1 <=> arr2; /* { dg-error "invalid operands" "" { target c++20 } } */
+#endif
+  return b;
+}
diff --git a/gcc/testsuite/c-c++-common/Warray-compare-2.c b/gcc/testsuite/c-c++-common/Warray-compare-2.c
new file mode 100644
index 00000000000..b3688e69b37
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Warray-compare-2.c
@@ -0,0 +1,44 @@ 
+/* PR c++/97573 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wno-array-compare" } */
+
+#ifndef __cplusplus
+# define bool _Bool
+#endif
+
+int arr1[5];
+int arr2[5];
+int arr3[2][2];
+int arr4[2][2];
+
+bool
+g ()
+{
+  bool b = arr1 == arr2; /* { dg-bogus "comparison between two arrays" } */
+  b &= arr1 != arr2; /* { dg-bogus "comparison between two arrays" } */
+  b &= arr1 > arr2; /* { dg-bogus "comparison between two arrays" } */
+  b &= arr1 >= arr2; /* { dg-bogus "comparison between two arrays" } */
+  b &= arr1 < arr2; /* { dg-bogus "comparison between two arrays" } */
+  b &= arr1 <= arr2; /* { dg-bogus "comparison between two arrays" } */
+#ifdef __cplusplus
+  b &= +arr1 == +arr2;
+  b &= +arr1 != +arr2;
+  b &= +arr1 > +arr2;
+  b &= +arr1 >= +arr2;
+  b &= +arr1 < +arr2;
+  b &= +arr1 <= +arr2;
+#endif
+  b &= &arr1[0] == &arr2[0];
+  b &= &arr1[0] != &arr2[0];
+  b &= &arr1[0] > &arr2[0];
+  b &= &arr1[0] >= &arr2[0];
+  b &= &arr1[0] < &arr2[0];
+  b &= &arr1[0] <= &arr2[0];
+
+  b &= arr3 == arr4; /* { dg-bogus "comparison between two arrays" } */
+
+#if defined(__cplusplus) && __cplusplus > 201703L
+  auto cmp = arr1 <=> arr2; /* { dg-error "invalid operands" "" { target c++20 } } */
+#endif
+  return b;
+}