On 14/02/2022, Jason Merrill <ja...@redhat.com> wrote: >> >> +/* Returns true if EXPR is a reference to an implicit >> + call to operator=(). */ >> +static bool >> +is_assignment_overload_ref_p (tree expr) >> +{ >> + if (expr == NULL_TREE || !REFERENCE_REF_P (expr)) >> + return false; > > This will only warn about op= that returns a reference, which is not > required. >
Ah I understand now. I added some new test cases for non-reference return types and copied some code from extract_call_expr that seems to do what we want. >> + return fndecl != NULL_TREE >> + && DECL_OVERLOADED_OPERATOR_P (fndecl) >> + && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR) >> + && DECL_ASSIGNMENT_OPERATOR_P (fndecl); > > This could be > > && DECL_ASSIGNMENT_OPERATOR_P (fndecl) > && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR); > > without the separate DECL_OVERLOADED_OPERATOR_P test. > I see. I've adjusted the code as you suggested and it works. Thanks! ------ Everything below is the patch v4 ------ 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 tested on x86_64-pc-linux-gnu. v3: https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590310.html Changes since v3: 1. Also handle COMPOUND_EXPRs and TARGET_EXPRs. v2: https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590236.html Changes since v2: 1. Add more test cases in Wparentheses-31.C. 2. Refactor added logic to a function (is_assignment_overload_ref_p). 3. Use REFERENCE_REF_P instead of INDIRECT_REF_P. v1: https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590158.html Changes since v1: 1. Use CALL_EXPR_OPERATOR_SYNTAX to avoid warnings for explicit operator=() calls. 2. Use INDIRECT_REF_P to filter implicit operator=() calls. 3. Use cp_get_callee_fndecl_nofold. 4. Add spaces before (. PR c/25689 gcc/cp/ChangeLog: * semantics.cc (maybe_convert_cond): Handle the implicit operator=() case for -Wparentheses. gcc/testsuite/ChangeLog: * g++.dg/warn/Wparentheses-31.C: New test. --- gcc/cp/semantics.cc | 29 +++++++++- gcc/testsuite/g++.dg/warn/Wparentheses-31.C | 62 +++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wparentheses-31.C diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index 0cb17a6a8ab1c..9c3f613a1aa62 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -815,6 +815,33 @@ 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 + to operator=() that is written as an operator expression. */ +static bool +is_assignment_op_expr_p(tree call) +{ + if (call == NULL_TREE) + return false; + + /* Unwrap the CALL_EXPR. */ + while (TREE_CODE (call) == COMPOUND_EXPR) + call = TREE_OPERAND (call, 1); + if (REFERENCE_REF_P (call)) + call = TREE_OPERAND (call, 0); + if (TREE_CODE (call) == TARGET_EXPR) + call = TARGET_EXPR_INITIAL (call); + + if (call == NULL_TREE + || TREE_CODE (call) != CALL_EXPR + || !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. */ @@ -836,7 +863,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 0000000000000..8d48ca5205782 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wparentheses-31.C @@ -0,0 +1,62 @@ +/* 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); +} + +/* 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)); + + /* Ideally, we wouldn't warn for non-empty classes using trivial + operator= (below), but we currently do as it is a MODIFY_EXPR. */ + // 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)); +} -- 2.35.1