c++: more ahead-of-time -Wparentheses warnings
Checks
Commit Message
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.
gcc/testsuite/ChangeLog:
* g++.dg/warn/Wparentheses-13.C: Strengthen by not requiring
that the templates are instantiated for any of the -Wparentheses
warnings to be issued.
* 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 | 22 +++++++++++----------
gcc/cp/typeck.cc | 7 ++++---
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, 21 insertions(+), 22 deletions(-)
Comments
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
@@ -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);
@@ -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),
@@ -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);
@@ -65,5 +65,3 @@ bar (T)
d = (a = a);
foo (27);
}
-
-template void bar<int> (int); // { dg-message "required" }
@@ -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);
@@ -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" }
}