On 2/14/22 21:30, Zhao Wei Liew wrote:
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.
Good plan, but let's call extract_call_expr rather than copy from it.
+ 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)
Missing space before (
+{
+ 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
