| Message ID | 20231025185556.844718-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: more ahead-of-time -Wparentheses warnings | expand |
On 10/25/23 14:55, Patrick Palka wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look > OK for trunk? > > -- >8 -- > > Now that we don't have to worry about looking thruogh NON_DEPENDENT_EXPR, > we can easily extend the -Wparentheses warning in convert_for_assignment > to consider (non-dependent) templated assignment operator expressions as > well, like r14-4111-g6e92a6a2a72d3b did in maybe_convert_cond. > > gcc/cp/ChangeLog: > > * cp-tree.h (is_assignment_op_expr_p): Declare. > * semantics.cc (is_assignment_op_expr_p): Generalize to return > true for assignment operator expression, not just one that > have been resolved to an operator overload. > (maybe_convert_cond): Remove now-redundant checks around > is_assignment_op_expr_p. > * typeck.cc (convert_for_assignment): Look through implicit > INDIRECT_REF in -Wparentheses warning logic, and generalize > to use is_assignment_op_expr_p. Do we want to factor out the whole warning logic rather than adjust it in both places? Jason
On Thu, 26 Oct 2023, Jason Merrill wrote: > On 10/25/23 14:55, Patrick Palka wrote: > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look > > OK for trunk? > > > > -- >8 -- > > > > Now that we don't have to worry about looking thruogh NON_DEPENDENT_EXPR, > > we can easily extend the -Wparentheses warning in convert_for_assignment > > to consider (non-dependent) templated assignment operator expressions as > > well, like r14-4111-g6e92a6a2a72d3b did in maybe_convert_cond. > > > > gcc/cp/ChangeLog: > > > > * cp-tree.h (is_assignment_op_expr_p): Declare. > > * semantics.cc (is_assignment_op_expr_p): Generalize to return > > true for assignment operator expression, not just one that > > have been resolved to an operator overload. > > (maybe_convert_cond): Remove now-redundant checks around > > is_assignment_op_expr_p. > > * typeck.cc (convert_for_assignment): Look through implicit > > INDIRECT_REF in -Wparentheses warning logic, and generalize > > to use is_assignment_op_expr_p. > > Do we want to factor out the whole warning logic rather than adjust it in both > places? Sounds good, like so? Bootstrap / regtest in progress. -- >8 -- Subject: [PATCH] c++: more ahead-of-time -Wparentheses warnings Now that we don't have to worry about looking through NON_DEPENDENT_EXPR, we can easily extend the -Wparentheses warning in convert_for_assignment to consider (non-dependent) templated assignment operator expressions as well, like r14-4111-g6e92a6a2a72d3b did in maybe_convert_cond. gcc/cp/ChangeLog: * cp-tree.h (maybe_warn_unparenthesized_assignment): Declare. * semantics.cc (is_assignment_op_expr_p): Generalize to return true for assignment operator expression, not just one that have been resolved to an operator overload. (maybe_warn_unparenthesized_assignment): Factored out from ... (maybe_convert_cond): ... here. (finish_parenthesized_expr): Also mention maybe_warn_unparenthesized_assignment. * typeck.cc (convert_for_assignment): Replace -Wparentheses warning logic with maybe_warn_unparenthesized_assignment. gcc/testsuite/ChangeLog: * g++.dg/warn/Wparentheses-13.C: Strengthen by expecting that the -Wparentheses warning are issued ahead of time. * g++.dg/warn/Wparentheses-23.C: Likewise. * g++.dg/warn/Wparentheses-32.C: Remove xfails. --- gcc/cp/cp-tree.h | 1 + gcc/cp/semantics.cc | 55 ++++++++++++++------- gcc/cp/typeck.cc | 13 ++--- gcc/testsuite/g++.dg/warn/Wparentheses-13.C | 2 - gcc/testsuite/g++.dg/warn/Wparentheses-23.C | 3 -- gcc/testsuite/g++.dg/warn/Wparentheses-32.C | 8 +-- 6 files changed, 44 insertions(+), 38 deletions(-) diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 30fe716b109..98b29e9cf81 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7875,6 +7875,7 @@ extern tree lambda_regenerating_args (tree); extern tree most_general_lambda (tree); extern tree finish_omp_target (location_t, tree, tree, bool); extern void finish_omp_target_clauses (location_t, tree, tree *); +extern void maybe_warn_unparenthesized_assignment (tree, tsubst_flags_t); /* in tree.cc */ extern int cp_tree_operand_length (const_tree); diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index 72ec72de690..5664da9f4f2 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -840,15 +840,20 @@ 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. */ +/* Returns true if T corresponds to an assignment operator expression. */ + static bool -is_assignment_op_expr_p (tree call) +is_assignment_op_expr_p (tree t) { - if (call == NULL_TREE) + if (t == NULL_TREE) return false; - call = extract_call_expr (call); + if (TREE_CODE (t) == MODIFY_EXPR + || (TREE_CODE (t) == MODOP_EXPR + && TREE_CODE (TREE_OPERAND (t, 1)) == NOP_EXPR)) + return true; + + tree call = extract_call_expr (t); if (call == NULL_TREE || call == error_mark_node || !CALL_EXPR_OPERATOR_SYNTAX (call)) @@ -860,6 +865,28 @@ is_assignment_op_expr_p (tree call) && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR); } +/* Maybe warn about an unparenthesized 'a = b' (appearing in a boolean + context). */ + +void +maybe_warn_unparenthesized_assignment (tree t, tsubst_flags_t complain) +{ + t = REFERENCE_REF_P (t) ? TREE_OPERAND (t, 0) : t; + + if ((complain & tf_warning) + && warn_parentheses + && is_assignment_op_expr_p (t) + /* A parenthesized expression would've gotten this warning + suppressed by finish_parenthesized_expr. */ + && !warning_suppressed_p (t, OPT_Wparentheses)) + { + warning_at (cp_expr_loc_or_input_loc (t), + OPT_Wparentheses, "suggest parentheses around " + "assignment used as truth value"); + suppress_warning (t, OPT_Wparentheses); + } +} + /* 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. */ @@ -878,21 +905,10 @@ maybe_convert_cond (tree cond) if (warn_sequence_point && !processing_template_decl) verify_sequence_points (cond); + maybe_warn_unparenthesized_assignment (cond, tf_warning_or_error); + /* Do the conversion. */ cond = convert_from_reference (cond); - - tree inner = REFERENCE_REF_P (cond) ? TREE_OPERAND (cond, 0) : cond; - if ((TREE_CODE (inner) == MODIFY_EXPR - || (TREE_CODE (inner) == MODOP_EXPR - && TREE_CODE (TREE_OPERAND (inner, 1)) == NOP_EXPR) - || is_assignment_op_expr_p (inner)) - && warn_parentheses - && !warning_suppressed_p (inner, OPT_Wparentheses) - && warning_at (cp_expr_loc_or_input_loc (inner), - OPT_Wparentheses, "suggest parentheses around " - "assignment used as truth value")) - suppress_warning (inner, OPT_Wparentheses); - return condition_conversion (cond); } @@ -2158,7 +2174,8 @@ finish_parenthesized_expr (cp_expr expr) { if (EXPR_P (expr)) { - /* This inhibits warnings in c_common_truthvalue_conversion. */ + /* This inhibits warnings in maybe_warn_unparenthesized_assignment + and c_common_truthvalue_conversion. */ tree inner = REFERENCE_REF_P (expr) ? TREE_OPERAND (expr, 0) : *expr; suppress_warning (inner, OPT_Wparentheses); } diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 00570f170ba..49afbd8fb5e 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -10338,16 +10338,9 @@ convert_for_assignment (tree type, tree rhs, /* If -Wparentheses, warn about a = b = c when a has type bool and b does not. */ - if (warn_parentheses - && TREE_CODE (type) == BOOLEAN_TYPE - && TREE_CODE (rhs) == MODIFY_EXPR - && !warning_suppressed_p (rhs, OPT_Wparentheses) - && TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE - && (complain & tf_warning) - && warning_at (rhs_loc, OPT_Wparentheses, - "suggest parentheses around assignment used as " - "truth value")) - suppress_warning (rhs, OPT_Wparentheses); + if (TREE_CODE (type) == BOOLEAN_TYPE + && TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE) + maybe_warn_unparenthesized_assignment (rhs, complain); if (complain & tf_warning) warn_for_address_or_pointer_of_packed_member (type, rhs); diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-13.C b/gcc/testsuite/g++.dg/warn/Wparentheses-13.C index 22a139f23a4..d6438942c28 100644 --- a/gcc/testsuite/g++.dg/warn/Wparentheses-13.C +++ b/gcc/testsuite/g++.dg/warn/Wparentheses-13.C @@ -65,5 +65,3 @@ bar (T) d = (a = a); foo (27); } - -template void bar<int> (int); // { dg-message "required" } diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-23.C b/gcc/testsuite/g++.dg/warn/Wparentheses-23.C index f1749c2b8da..bd7195e5023 100644 --- a/gcc/testsuite/g++.dg/warn/Wparentheses-23.C +++ b/gcc/testsuite/g++.dg/warn/Wparentheses-23.C @@ -114,8 +114,5 @@ bar4 (T) return (a = a); } -template void bar<int> (int); // { dg-message "required" } -template bool bar1<int> (int); // { dg-message "required" } template bool bar2<int> (int); -template bool bar3<int> (int); // { dg-message "required" } template bool bar4<int> (int); diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-32.C b/gcc/testsuite/g++.dg/warn/Wparentheses-32.C index 719a9d9e73a..b03515afd55 100644 --- a/gcc/testsuite/g++.dg/warn/Wparentheses-32.C +++ b/gcc/testsuite/g++.dg/warn/Wparentheses-32.C @@ -21,8 +21,8 @@ void f() { if (z1 = z2) { } // { dg-warning "parentheses" } bool b; - b = m = n; // { dg-warning "parentheses" "" { xfail *-*-* } } - b = x1 = x2; // { dg-warning "parentheses" "" { xfail *-*-* } } - b = y1 = y2; // { dg-warning "parentheses" "" { xfail *-*-* } } - b = z1 = z2; // { dg-warning "parentheses" "" { xfail *-*-* } } + b = m = n; // { dg-warning "parentheses" } + b = x1 = x2; // { dg-warning "parentheses" } + b = y1 = y2; // { dg-warning "parentheses" } + b = z1 = z2; // { dg-warning "parentheses" } }
On Thu, 26 Oct 2023, Patrick Palka wrote: > On Thu, 26 Oct 2023, Jason Merrill wrote: > > > On 10/25/23 14:55, Patrick Palka wrote: > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look > > > OK for trunk? > > > > > > -- >8 -- > > > > > > Now that we don't have to worry about looking thruogh NON_DEPENDENT_EXPR, > > > we can easily extend the -Wparentheses warning in convert_for_assignment > > > to consider (non-dependent) templated assignment operator expressions as > > > well, like r14-4111-g6e92a6a2a72d3b did in maybe_convert_cond. > > > > > > gcc/cp/ChangeLog: > > > > > > * cp-tree.h (is_assignment_op_expr_p): Declare. > > > * semantics.cc (is_assignment_op_expr_p): Generalize to return > > > true for assignment operator expression, not just one that > > > have been resolved to an operator overload. > > > (maybe_convert_cond): Remove now-redundant checks around > > > is_assignment_op_expr_p. > > > * typeck.cc (convert_for_assignment): Look through implicit > > > INDIRECT_REF in -Wparentheses warning logic, and generalize > > > to use is_assignment_op_expr_p. > > > > Do we want to factor out the whole warning logic rather than adjust it in both > > places? > > Sounds good, like so? Bootstrap / regtest in progress. > > -- >8 -- > > Subject: [PATCH] c++: more ahead-of-time -Wparentheses warnings > > Now that we don't have to worry about looking through NON_DEPENDENT_EXPR, > we can easily extend the -Wparentheses warning in convert_for_assignment > to consider (non-dependent) templated assignment operator expressions as > well, like r14-4111-g6e92a6a2a72d3b did in maybe_convert_cond. > > gcc/cp/ChangeLog: > > * cp-tree.h (maybe_warn_unparenthesized_assignment): Declare. > * semantics.cc (is_assignment_op_expr_p): Generalize to return > true for assignment operator expression, not just one that > have been resolved to an operator overload. > (maybe_warn_unparenthesized_assignment): Factored out from ... > (maybe_convert_cond): ... here. > (finish_parenthesized_expr): Also mention > maybe_warn_unparenthesized_assignment. > * typeck.cc (convert_for_assignment): Replace -Wparentheses > warning logic with maybe_warn_unparenthesized_assignment. > > gcc/testsuite/ChangeLog: > > * g++.dg/warn/Wparentheses-13.C: Strengthen by expecting that > the -Wparentheses warning are issued ahead of time. > * g++.dg/warn/Wparentheses-23.C: Likewise. > * g++.dg/warn/Wparentheses-32.C: Remove xfails. > --- > gcc/cp/cp-tree.h | 1 + > gcc/cp/semantics.cc | 55 ++++++++++++++------- > gcc/cp/typeck.cc | 13 ++--- > gcc/testsuite/g++.dg/warn/Wparentheses-13.C | 2 - > gcc/testsuite/g++.dg/warn/Wparentheses-23.C | 3 -- > gcc/testsuite/g++.dg/warn/Wparentheses-32.C | 8 +-- > 6 files changed, 44 insertions(+), 38 deletions(-) > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index 30fe716b109..98b29e9cf81 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -7875,6 +7875,7 @@ extern tree lambda_regenerating_args (tree); > extern tree most_general_lambda (tree); > extern tree finish_omp_target (location_t, tree, tree, bool); > extern void finish_omp_target_clauses (location_t, tree, tree *); > +extern void maybe_warn_unparenthesized_assignment (tree, tsubst_flags_t); > > /* in tree.cc */ > extern int cp_tree_operand_length (const_tree); > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc > index 72ec72de690..5664da9f4f2 100644 > --- a/gcc/cp/semantics.cc > +++ b/gcc/cp/semantics.cc > @@ -840,15 +840,20 @@ 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. */ > +/* Returns true if T corresponds to an assignment operator expression. */ > + > static bool > -is_assignment_op_expr_p (tree call) > +is_assignment_op_expr_p (tree t) > { > - if (call == NULL_TREE) > + if (t == NULL_TREE) > return false; > > - call = extract_call_expr (call); > + if (TREE_CODE (t) == MODIFY_EXPR > + || (TREE_CODE (t) == MODOP_EXPR > + && TREE_CODE (TREE_OPERAND (t, 1)) == NOP_EXPR)) > + return true; > + > + tree call = extract_call_expr (t); > if (call == NULL_TREE > || call == error_mark_node > || !CALL_EXPR_OPERATOR_SYNTAX (call)) > @@ -860,6 +865,28 @@ is_assignment_op_expr_p (tree call) > && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR); > } > > +/* Maybe warn about an unparenthesized 'a = b' (appearing in a boolean > + context). */ > + > +void > +maybe_warn_unparenthesized_assignment (tree t, tsubst_flags_t complain) > +{ > + t = REFERENCE_REF_P (t) ? TREE_OPERAND (t, 0) : t; Consider this to instead be written more idiomatically as if (REFERENCE_REF_P (t)) t = TREE_OPERAND (t, 0); > + > + if ((complain & tf_warning) > + && warn_parentheses > + && is_assignment_op_expr_p (t) > + /* A parenthesized expression would've gotten this warning > + suppressed by finish_parenthesized_expr. */ > + && !warning_suppressed_p (t, OPT_Wparentheses)) > + { > + warning_at (cp_expr_loc_or_input_loc (t), > + OPT_Wparentheses, "suggest parentheses around " > + "assignment used as truth value"); > + suppress_warning (t, OPT_Wparentheses); > + } > +} > + > /* 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. */ > @@ -878,21 +905,10 @@ maybe_convert_cond (tree cond) > if (warn_sequence_point && !processing_template_decl) > verify_sequence_points (cond); > > + maybe_warn_unparenthesized_assignment (cond, tf_warning_or_error); > + > /* Do the conversion. */ > cond = convert_from_reference (cond); > - > - tree inner = REFERENCE_REF_P (cond) ? TREE_OPERAND (cond, 0) : cond; > - if ((TREE_CODE (inner) == MODIFY_EXPR > - || (TREE_CODE (inner) == MODOP_EXPR > - && TREE_CODE (TREE_OPERAND (inner, 1)) == NOP_EXPR) > - || is_assignment_op_expr_p (inner)) > - && warn_parentheses > - && !warning_suppressed_p (inner, OPT_Wparentheses) > - && warning_at (cp_expr_loc_or_input_loc (inner), > - OPT_Wparentheses, "suggest parentheses around " > - "assignment used as truth value")) > - suppress_warning (inner, OPT_Wparentheses); > - > return condition_conversion (cond); > } > > @@ -2158,7 +2174,8 @@ finish_parenthesized_expr (cp_expr expr) > { > if (EXPR_P (expr)) > { > - /* This inhibits warnings in c_common_truthvalue_conversion. */ > + /* This inhibits warnings in maybe_warn_unparenthesized_assignment > + and c_common_truthvalue_conversion. */ > tree inner = REFERENCE_REF_P (expr) ? TREE_OPERAND (expr, 0) : *expr; > suppress_warning (inner, OPT_Wparentheses); > } > diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc > index 00570f170ba..49afbd8fb5e 100644 > --- a/gcc/cp/typeck.cc > +++ b/gcc/cp/typeck.cc > @@ -10338,16 +10338,9 @@ convert_for_assignment (tree type, tree rhs, > > /* If -Wparentheses, warn about a = b = c when a has type bool and b > does not. */ > - if (warn_parentheses > - && TREE_CODE (type) == BOOLEAN_TYPE > - && TREE_CODE (rhs) == MODIFY_EXPR > - && !warning_suppressed_p (rhs, OPT_Wparentheses) > - && TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE > - && (complain & tf_warning) > - && warning_at (rhs_loc, OPT_Wparentheses, > - "suggest parentheses around assignment used as " > - "truth value")) > - suppress_warning (rhs, OPT_Wparentheses); > + if (TREE_CODE (type) == BOOLEAN_TYPE > + && TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE) > + maybe_warn_unparenthesized_assignment (rhs, complain); > > if (complain & tf_warning) > warn_for_address_or_pointer_of_packed_member (type, rhs); > diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-13.C b/gcc/testsuite/g++.dg/warn/Wparentheses-13.C > index 22a139f23a4..d6438942c28 100644 > --- a/gcc/testsuite/g++.dg/warn/Wparentheses-13.C > +++ b/gcc/testsuite/g++.dg/warn/Wparentheses-13.C > @@ -65,5 +65,3 @@ bar (T) > d = (a = a); > foo (27); > } > - > -template void bar<int> (int); // { dg-message "required" } > diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-23.C b/gcc/testsuite/g++.dg/warn/Wparentheses-23.C > index f1749c2b8da..bd7195e5023 100644 > --- a/gcc/testsuite/g++.dg/warn/Wparentheses-23.C > +++ b/gcc/testsuite/g++.dg/warn/Wparentheses-23.C > @@ -114,8 +114,5 @@ bar4 (T) > return (a = a); > } > > -template void bar<int> (int); // { dg-message "required" } > -template bool bar1<int> (int); // { dg-message "required" } > template bool bar2<int> (int); > -template bool bar3<int> (int); // { dg-message "required" } > template bool bar4<int> (int); > diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-32.C b/gcc/testsuite/g++.dg/warn/Wparentheses-32.C > index 719a9d9e73a..b03515afd55 100644 > --- a/gcc/testsuite/g++.dg/warn/Wparentheses-32.C > +++ b/gcc/testsuite/g++.dg/warn/Wparentheses-32.C > @@ -21,8 +21,8 @@ void f() { > if (z1 = z2) { } // { dg-warning "parentheses" } > > bool b; > - b = m = n; // { dg-warning "parentheses" "" { xfail *-*-* } } > - b = x1 = x2; // { dg-warning "parentheses" "" { xfail *-*-* } } > - b = y1 = y2; // { dg-warning "parentheses" "" { xfail *-*-* } } > - b = z1 = z2; // { dg-warning "parentheses" "" { xfail *-*-* } } > + b = m = n; // { dg-warning "parentheses" } > + b = x1 = x2; // { dg-warning "parentheses" } > + b = y1 = y2; // { dg-warning "parentheses" } > + b = z1 = z2; // { dg-warning "parentheses" } > } > -- > 2.42.0.482.g2e8e77cbac > >
On 10/26/23 17:37, Patrick Palka wrote: > On Thu, 26 Oct 2023, Patrick Palka wrote: > >> On Thu, 26 Oct 2023, Jason Merrill wrote: >> >>> On 10/25/23 14:55, Patrick Palka wrote: >>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look >>>> OK for trunk? >>>> >>>> -- >8 -- >>>> >>>> Now that we don't have to worry about looking thruogh NON_DEPENDENT_EXPR, >>>> we can easily extend the -Wparentheses warning in convert_for_assignment >>>> to consider (non-dependent) templated assignment operator expressions as >>>> well, like r14-4111-g6e92a6a2a72d3b did in maybe_convert_cond. >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> * cp-tree.h (is_assignment_op_expr_p): Declare. >>>> * semantics.cc (is_assignment_op_expr_p): Generalize to return >>>> true for assignment operator expression, not just one that >>>> have been resolved to an operator overload. >>>> (maybe_convert_cond): Remove now-redundant checks around >>>> is_assignment_op_expr_p. >>>> * typeck.cc (convert_for_assignment): Look through implicit >>>> INDIRECT_REF in -Wparentheses warning logic, and generalize >>>> to use is_assignment_op_expr_p. >>> >>> Do we want to factor out the whole warning logic rather than adjust it in both >>> places? >> >> Sounds good, like so? Bootstrap / regtest in progress. >> >> -- >8 -- >> >> Subject: [PATCH] c++: more ahead-of-time -Wparentheses warnings >> >> Now that we don't have to worry about looking through NON_DEPENDENT_EXPR, >> we can easily extend the -Wparentheses warning in convert_for_assignment >> to consider (non-dependent) templated assignment operator expressions as >> well, like r14-4111-g6e92a6a2a72d3b did in maybe_convert_cond. >> >> gcc/cp/ChangeLog: >> >> * cp-tree.h (maybe_warn_unparenthesized_assignment): Declare. >> * semantics.cc (is_assignment_op_expr_p): Generalize to return >> true for assignment operator expression, not just one that >> have been resolved to an operator overload. >> (maybe_warn_unparenthesized_assignment): Factored out from ... >> (maybe_convert_cond): ... here. >> (finish_parenthesized_expr): Also mention >> maybe_warn_unparenthesized_assignment. >> * typeck.cc (convert_for_assignment): Replace -Wparentheses >> warning logic with maybe_warn_unparenthesized_assignment. >> >> gcc/testsuite/ChangeLog: >> >> * g++.dg/warn/Wparentheses-13.C: Strengthen by expecting that >> the -Wparentheses warning are issued ahead of time. >> * g++.dg/warn/Wparentheses-23.C: Likewise. >> * g++.dg/warn/Wparentheses-32.C: Remove xfails. >> --- >> gcc/cp/cp-tree.h | 1 + >> gcc/cp/semantics.cc | 55 ++++++++++++++------- >> gcc/cp/typeck.cc | 13 ++--- >> gcc/testsuite/g++.dg/warn/Wparentheses-13.C | 2 - >> gcc/testsuite/g++.dg/warn/Wparentheses-23.C | 3 -- >> gcc/testsuite/g++.dg/warn/Wparentheses-32.C | 8 +-- >> 6 files changed, 44 insertions(+), 38 deletions(-) >> >> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h >> index 30fe716b109..98b29e9cf81 100644 >> --- a/gcc/cp/cp-tree.h >> +++ b/gcc/cp/cp-tree.h >> @@ -7875,6 +7875,7 @@ extern tree lambda_regenerating_args (tree); >> extern tree most_general_lambda (tree); >> extern tree finish_omp_target (location_t, tree, tree, bool); >> extern void finish_omp_target_clauses (location_t, tree, tree *); >> +extern void maybe_warn_unparenthesized_assignment (tree, tsubst_flags_t); >> >> /* in tree.cc */ >> extern int cp_tree_operand_length (const_tree); >> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc >> index 72ec72de690..5664da9f4f2 100644 >> --- a/gcc/cp/semantics.cc >> +++ b/gcc/cp/semantics.cc >> @@ -840,15 +840,20 @@ 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. */ >> +/* Returns true if T corresponds to an assignment operator expression. */ >> + >> static bool >> -is_assignment_op_expr_p (tree call) >> +is_assignment_op_expr_p (tree t) >> { >> - if (call == NULL_TREE) >> + if (t == NULL_TREE) >> return false; >> >> - call = extract_call_expr (call); >> + if (TREE_CODE (t) == MODIFY_EXPR >> + || (TREE_CODE (t) == MODOP_EXPR >> + && TREE_CODE (TREE_OPERAND (t, 1)) == NOP_EXPR)) >> + return true; >> + >> + tree call = extract_call_expr (t); >> if (call == NULL_TREE >> || call == error_mark_node >> || !CALL_EXPR_OPERATOR_SYNTAX (call)) >> @@ -860,6 +865,28 @@ is_assignment_op_expr_p (tree call) >> && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR); >> } >> >> +/* Maybe warn about an unparenthesized 'a = b' (appearing in a boolean >> + context). */ >> + >> +void >> +maybe_warn_unparenthesized_assignment (tree t, tsubst_flags_t complain) >> +{ >> + t = REFERENCE_REF_P (t) ? TREE_OPERAND (t, 0) : t; > > Consider this to instead be written more idiomatically as OK. Jason
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 30fe716b109..c90ef883e52 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7875,6 +7875,7 @@ extern tree lambda_regenerating_args (tree); extern tree most_general_lambda (tree); extern tree finish_omp_target (location_t, tree, tree, bool); extern void finish_omp_target_clauses (location_t, tree, tree *); +extern bool is_assignment_op_expr_p (tree); /* in tree.cc */ extern int cp_tree_operand_length (const_tree); diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index 72ec72de690..4b0038a4fc7 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -840,15 +840,20 @@ 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) +/* Returns true if T corresponds to an assignment operator expression. */ + +bool +is_assignment_op_expr_p (tree t) { - if (call == NULL_TREE) + if (t == NULL_TREE) return false; - call = extract_call_expr (call); + if (TREE_CODE (t) == MODIFY_EXPR + || (TREE_CODE (t) == MODOP_EXPR + && TREE_CODE (TREE_OPERAND (t, 1)) == NOP_EXPR)) + return true; + + tree call = extract_call_expr (t); if (call == NULL_TREE || call == error_mark_node || !CALL_EXPR_OPERATOR_SYNTAX (call)) @@ -882,10 +887,7 @@ maybe_convert_cond (tree cond) cond = convert_from_reference (cond); tree inner = REFERENCE_REF_P (cond) ? TREE_OPERAND (cond, 0) : cond; - if ((TREE_CODE (inner) == MODIFY_EXPR - || (TREE_CODE (inner) == MODOP_EXPR - && TREE_CODE (TREE_OPERAND (inner, 1)) == NOP_EXPR) - || is_assignment_op_expr_p (inner)) + if (is_assignment_op_expr_p (inner) && warn_parentheses && !warning_suppressed_p (inner, OPT_Wparentheses) && warning_at (cp_expr_loc_or_input_loc (inner), diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 3b719326d76..0585b4a6bf0 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -10338,16 +10338,17 @@ convert_for_assignment (tree type, tree rhs, /* If -Wparentheses, warn about a = b = c when a has type bool and b does not. */ + tree inner_rhs = REFERENCE_REF_P (rhs) ? TREE_OPERAND (rhs, 0) : rhs; if (warn_parentheses && TREE_CODE (type) == BOOLEAN_TYPE - && TREE_CODE (rhs) == MODIFY_EXPR - && !warning_suppressed_p (rhs, OPT_Wparentheses) + && is_assignment_op_expr_p (inner_rhs) + && !warning_suppressed_p (inner_rhs, OPT_Wparentheses) && TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE && (complain & tf_warning) && warning_at (rhs_loc, OPT_Wparentheses, "suggest parentheses around assignment used as " "truth value")) - suppress_warning (rhs, OPT_Wparentheses); + suppress_warning (inner_rhs, OPT_Wparentheses); if (complain & tf_warning) warn_for_address_or_pointer_of_packed_member (type, rhs); diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-13.C b/gcc/testsuite/g++.dg/warn/Wparentheses-13.C index 22a139f23a4..d6438942c28 100644 --- a/gcc/testsuite/g++.dg/warn/Wparentheses-13.C +++ b/gcc/testsuite/g++.dg/warn/Wparentheses-13.C @@ -65,5 +65,3 @@ bar (T) d = (a = a); foo (27); } - -template void bar<int> (int); // { dg-message "required" } diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-23.C b/gcc/testsuite/g++.dg/warn/Wparentheses-23.C index f1749c2b8da..bd7195e5023 100644 --- a/gcc/testsuite/g++.dg/warn/Wparentheses-23.C +++ b/gcc/testsuite/g++.dg/warn/Wparentheses-23.C @@ -114,8 +114,5 @@ bar4 (T) return (a = a); } -template void bar<int> (int); // { dg-message "required" } -template bool bar1<int> (int); // { dg-message "required" } template bool bar2<int> (int); -template bool bar3<int> (int); // { dg-message "required" } template bool bar4<int> (int); diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-32.C b/gcc/testsuite/g++.dg/warn/Wparentheses-32.C index 719a9d9e73a..b03515afd55 100644 --- a/gcc/testsuite/g++.dg/warn/Wparentheses-32.C +++ b/gcc/testsuite/g++.dg/warn/Wparentheses-32.C @@ -21,8 +21,8 @@ void f() { if (z1 = z2) { } // { dg-warning "parentheses" } bool b; - b = m = n; // { dg-warning "parentheses" "" { xfail *-*-* } } - b = x1 = x2; // { dg-warning "parentheses" "" { xfail *-*-* } } - b = y1 = y2; // { dg-warning "parentheses" "" { xfail *-*-* } } - b = z1 = z2; // { dg-warning "parentheses" "" { xfail *-*-* } } + b = m = n; // { dg-warning "parentheses" } + b = x1 = x2; // { dg-warning "parentheses" } + b = y1 = y2; // { dg-warning "parentheses" } + b = z1 = z2; // { dg-warning "parentheses" } }