[gcc r13-23] c++: Add diagnostic when operator= is used as truth cond [PR25689]

[gcc r13-23] c++: Add diagnostic when operator= is used as truth cond [PR25689]

[Jason Merrill]

https://gcc.gnu.org/g:654f6978cdc85a3970ff2c478d4df3e55cf4d3ab

commit r13-23-g654f6978cdc85a3970ff2c478d4df3e55cf4d3ab
Author: Zhao Wei Liew <zhaoweiliew@gmail.com>
Date:   Tue Feb 15 17:44:29 2022 +0800

    c++: Add diagnostic when operator= is used as truth cond [PR25689]
    
    When compiling the following code with g++ -Wparentheses, GCC does not
    warn on the if statement. For example, there is no warning for this code:
    
    struct A {
            A& operator=(int);
            operator bool();
    };
    
    void f(A a) {
            if (a = 0); // no warning
    }
    
    This is because a = 0 is a call to operator=, which GCC does not handle.
    
    This patch fixes this issue by handling calls to operator= when deciding
    to warn.
    
    Bootstrapped and regression tested on x86_64-pc-linux-gnu.
    
            PR c++/25689
    
    gcc/cp/ChangeLog:
    
            * call.cc (extract_call_expr): Return a NULL_TREE on failure
            instead of asserting.
            (build_new_method_call): Suppress -Wparentheses diagnostic for
            MODIFY_EXPR.
            * semantics.cc (is_assignment_op_expr_p): Add function to check
            if an expression is a call to an op= operator expression.
            (maybe_convert_cond): Handle the case of a op= operator expression
            for the -Wparentheses diagnostic.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/warn/Wparentheses-31.C: New test.
    
    Signed-off-by: Zhao Wei Liew <zhaoweiliew@gmail.com>

Diff:
---
 gcc/cp/call.cc                              | 13 +++++--
 gcc/cp/semantics.cc                         | 22 ++++++++++-
 gcc/testsuite/g++.dg/warn/Wparentheses-31.C | 59 +++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 4 deletions(-)

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index fa18d7f8f9d..959279d6216 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -7111,9 +7111,10 @@ extract_call_expr (tree call)
       default:;
       }
 
-  gcc_assert (TREE_CODE (call) == CALL_EXPR
-	      || TREE_CODE (call) == AGGR_INIT_EXPR
-	      || call == error_mark_node);
+  if (TREE_CODE (call) != CALL_EXPR
+      && TREE_CODE (call) != AGGR_INIT_EXPR
+      && call != error_mark_node)
+    return NULL_TREE;
   return call;
 }
 
@@ -11180,6 +11181,12 @@ build_new_method_call (tree instance, tree fns, vec<tree, va_gc> **args,
 		*fn_p = fn;
 	      /* Build the actual CALL_EXPR.  */
 	      call = build_over_call (cand, flags, complain);
+
+	      /* Suppress warnings for if (my_struct.operator= (x)) where
+		 my_struct is implicitly converted to bool. */
+	      if (TREE_CODE (call) == MODIFY_EXPR)
+		suppress_warning (call, OPT_Wparentheses);
+
 	      /* In an expression of the form `a->f()' where `f' turns
 		 out to be a static member function, `a' is
 		 none-the-less evaluated.  */
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index ab48f11c9be..9567559c962 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -826,6 +826,26 @@ finish_goto_stmt (tree destination)
   return add_stmt (build_stmt (input_location, GOTO_EXPR, destination));
 }
 
+/* Returns true if CALL is a (possibly wrapped) CALL_EXPR or AGGR_INIT_EXPR
+   to operator= () that is written as an operator expression. */
+static bool
+is_assignment_op_expr_p (tree call)
+{
+  if (call == NULL_TREE)
+    return false;
+
+  call = extract_call_expr (call);
+  if (call == NULL_TREE
+      || call == error_mark_node
+      || !CALL_EXPR_OPERATOR_SYNTAX (call))
+    return false;
+
+  tree fndecl = cp_get_callee_fndecl_nofold (call);
+  return fndecl != NULL_TREE
+    && DECL_ASSIGNMENT_OPERATOR_P (fndecl)
+    && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR);
+}
+
 /* COND is the condition-expression for an if, while, etc.,
    statement.  Convert it to a boolean value, if appropriate.
    In addition, verify sequence points if -Wsequence-point is enabled.  */
@@ -847,7 +867,7 @@ maybe_convert_cond (tree cond)
   /* Do the conversion.  */
   cond = convert_from_reference (cond);
 
-  if (TREE_CODE (cond) == MODIFY_EXPR
+  if ((TREE_CODE (cond) == MODIFY_EXPR || is_assignment_op_expr_p (cond))
       && warn_parentheses
       && !warning_suppressed_p (cond, OPT_Wparentheses)
       && warning_at (cp_expr_loc_or_input_loc (cond),
diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-31.C b/gcc/testsuite/g++.dg/warn/Wparentheses-31.C
new file mode 100644
index 00000000000..f29234ce589
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wparentheses-31.C
@@ -0,0 +1,59 @@
+/* Test that -Wparentheses warns for struct/class assignments,
+   except for explicit calls to operator= (). */
+/* PR c++/25689 */
+/* { dg-options "-Wparentheses" }  */
+
+struct A
+{
+	A& operator= (int);
+	A operator= (double);
+	operator bool ();
+};
+
+struct B
+{
+	bool x;
+	B& operator= (int);
+	B operator= (double);
+	operator bool ();
+};
+
+struct C
+{
+	C& operator= (int);
+	virtual C operator= (double);
+	operator bool ();
+};
+
+/* Test empty class */
+void f1 (A a1, A a2)
+{
+	if (a1 = 0); /* { dg-warning "suggest parentheses" } */
+	if (a1 = 0.); /* { dg-warning "suggest parentheses" } */
+	if (a1.operator= (0));
+	if (a1.operator= (a2));
+
+	/* Ideally, we'd warn for empty classes using trivial operator= (below),
+	   but we don't do so yet as it is a non-trivial COMPOUND_EXPR. */
+	if (a1 = a2); /* { dg-warning "suggest parentheses" "" { xfail *-*-* } } */
+}
+
+/* Test non-empty class */
+void f2 (B b1, B b2)
+{
+	if (b1 = 0); /* { dg-warning "suggest parentheses" } */
+	if (b1 = 0.); /* { dg-warning "suggest parentheses" } */
+	if (b1 = b2); /* { dg-warning "suggest parentheses" } */
+	if (b1.operator= (0));
+	if (b1.operator= (b2));
+}
+
+/* Test class with vtable */
+void f3 (C c1, C c2)
+{
+	if (c1 = 0); /* { dg-warning "suggest parentheses" } */
+	if (c1 = 0.); /* { dg-warning "suggest parentheses" } */
+	if (c1 = c2); /* { dg-warning "suggest parentheses" } */
+	if (c1.operator= (0));
+	if (c1.operator= (c2));
+}