On 2/9/22 21:18, Zhao Wei Liew via Gcc-patches wrote:
Hi!
I wrote a patch for PR 25689, but I feel like it may not be the ideal
fix. Furthermore, there are some standing issues with the patch for
which I would like tips on how to fix them.
Specifically, there are 2 issues:
1. GCC warns about if (a.operator=(0)). That said, this may not be a
major issue as I don't think such code is widely written.
Can you avoid this by checking CALL_EXPR_OPERATOR_SYNTAX?
2. GCC does not warn for `if (a = b)` where the default copy/move
assignment operator is used.
The code for trivial copy-assignment should be pretty recognizable, as a
MODIFY_EXPR of two MEM_REFs; it's built in build_over_call after the
comment "We must only copy the non-tail padding parts."
I've included a code snippet in PR25689 that shows the 2 issues I
mentioned. I appreciate any feedback, thanks!
----Everything below is the actual patch----
When compiling the following code with g++ -Wparentheses, GCC does not
warn on the if statement:
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 check
for.
This patch fixes that by checking for calls to operator= when deciding
to warn.
PR c/25689
gcc/cp/ChangeLog:
* semantics.cc (maybe_convert_cond): Handle the operator=() case
as well.
gcc/testsuite/ChangeLog:
* g++.dg/warn/Wparentheses-31.C: New test.
---
gcc/cp/semantics.cc | 14 +++++++++++++-
gcc/testsuite/g++.dg/warn/Wparentheses-31.C | 11 +++++++++++
2 files changed, 24 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 466d6b56871f4..6a25d039585f2 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -836,7 +836,19 @@ maybe_convert_cond (tree cond)
/* Do the conversion. */
cond = convert_from_reference (cond);
- if (TREE_CODE (cond) == MODIFY_EXPR
+ /* Also check if this is a call to operator=().
+ Example: if (my_struct = 5) {...}
+ */
+ tree fndecl = NULL_TREE;
+ if (TREE_OPERAND_LENGTH(cond) >= 1) {
+ fndecl = cp_get_callee_fndecl(TREE_OPERAND(cond, 0));
Let's use cp_get_callee_fndecl_nofold.
Please add a space before all (
+ }
+
+ if ((TREE_CODE (cond) == MODIFY_EXPR
+ || (fndecl != NULL_TREE
+ && DECL_OVERLOADED_OPERATOR_P(fndecl)
+ && DECL_OVERLOADED_OPERATOR_IS(fndecl, NOP_EXPR)
+ && DECL_ASSIGNMENT_OPERATOR_P(fndecl)))
&& 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..abd7476ccb461
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wparentheses-31.C
@@ -0,0 +1,11 @@
+/* PR c/25689 */
+/* { dg-options "-Wparentheses" } */
+
+struct A {
+ A& operator=(int);
+ operator bool();
+};
+
+void f(A a) {
+ if (a = 0); /* { dg-warning "suggest parentheses" } */
+}
--
2.17.1
