Re: [PATCH v2] c-family: Implement -Warray-compare [PR97573]

2021-10-03 Thread Jeff Law via Gcc-patches




On 10/1/2021 5:08 PM, Marek Polacek via Gcc-patches wrote:

On Fri, Oct 01, 2021 at 11:15:45PM +0200, Jakub Jelinek wrote:

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.

Thanks, fixed:

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

-- >8 --
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.

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.

OK.  It'll be interesting to see how many dusty packages trip over this ;-)

jeff



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

2021-10-01 Thread Marek Polacek via Gcc-patches
On Fri, Oct 01, 2021 at 11:15:45PM +0200, Jakub Jelinek wrote:
> 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.

Thanks, fixed:

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

-- >8 --
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.

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

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..99cde4a2a59 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,
+ (c_dialect_cxx () && cxx_dialect >= cxx20)
+ ? G_("comparison between two arrays is deprecated in C++20")
+ : G_("comparison between two arrays")))
+{
+  /* 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
---