On 2/17/22 23:30, Zhao Wei Liew wrote:
On Fri, 18 Feb 2022 at 08:32, Zhao Wei Liew <zhaoweil...@gmail.com> wrote:
+/* 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));
You can avoid it by calling suppress_warning on that MODIFY_EXPR in
build_over_call.
Unfortunately, that also affects the warning for if (b1 = b2) just 5
lines above. Both expressions seem to generate the same tree structure.
True, you would need to put the call to suppress_warning in build_new_op
around where CALL_EXPR_OPERATOR_SYNTAX is set.
It seems like that would suppress the warning for the case of if (b1 = b2)
instead of
if (b1.operator= (b2)). Do you mean to add the call to suppress_warning
in build_method_call instead?
This is what I've tried so far:
1. Call suppress_warning (result, ...) in the trivial_fn_p block in
build_new_op,
right above the comment "There won't be a CALL_EXPR" (line 6699).
This suppresses the warning for if (b1 = b2) but not for if (b1.operator=
(b2)).
2. Call suppress_warning (result, ...) in build_method_call, right after the
call to
build_over_call (line 11141). This suppresses the warning for if
(b1.operator= (b2))
and not if (b1 = b2).
Based on this, I think the 2nd option might be what we want here? Please
correct me if I'm
wrong. I'm also unsure if there are issues that might arise with this change.
To better illustrate the 2nd option, I've attached it as a patch v8.
How does it look?
It looks good, but unfortunately regresses some other warning tests,
such as Wnonnull5.C. Please remember to run the regression tests before
sending a patch (https://gcc.gnu.org/contribute.html#testing).
This seems to be a complicated problem with suppress_warning, which
means your call to suppress_warning effectively silences all later
warnings, not just -Wparentheses.
You should be able to work around this issue by only calling
suppress_warning in the specific case we're interested in, i.e. when
warn_parentheses is enabled and "call" is a MODIFY_EXPR.
v7: https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590464.html
Changes since v7:
1. Suppress -Wparentheses warnings in build_new_method_call.
2. Uncomment the test case for if (b1.operator= (b2)).
v6: https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590419.html
Changes since v6:
1. Check for error_mark_node in is_assignment_op_expr_pr.
2. Change "c:" to "c++:".
v5: https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590393.html
Changes since v5:
1. Revert changes in v4.
2. Replace gcc_assert with a return NULL_TREE in extract_call_expr.
v4: https://gcc.gnu.org/pipermail/gcc-patches/2022-February/590379.html
Changes since v4:
1. Refactor the non-assert-related code out of extract_call_expr and
call that function instead to check for call expressions.
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 (.
