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" }
}
--
2.42.0.482.g2e8e77cbac
