c++: lvalueness of non-dependent assignment [PR114994]
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk/14? For trunk as a follow-up I can implement the
mentionted representation change to use CALL_EXPR instead of
MODOP_EXPR for a non-dependent simple assignment expression that
resolved to an operator= overload.
-- >8 --
r14-4111 made us check non-dependent assignment expressions ahead of
time, as well as give them a type. Unlike for compound assignment
expressions however, if a simple assignment resolves to an operator
overload we still represent it as a (typed) MODOP_EXPR instead of a
CALL_EXPR to the selected overload. This, I reckoned, was just a
pessimization (since we'll have to repeat overload resolution at
instantiatiation time) but should be harmless. (And it should be
easily fixable by giving cp_build_modify_expr an 'overload' parameter).
But it breaks the below testcase ultimately because MODOP_EXPR (of
non-reference type) is always treated as an lvalue according to
lvalue_kind, which is incorrect for the MODOP_EXPR representing x=42.
We can fix this by representing such assignment expressions as CALL_EXPRs
matching what that of compound assignments, but that turns out to
require some tweaking of our -Wparentheses warning logic which seems
unsuitable for backporting.
So this patch instead more conservatively fixes this by refining
lvalue_kind to consider the type of a (simple) MODOP_EXPR as we
already do for COND_EXPR.
PR c++/114994
gcc/cp/ChangeLog:
* tree.cc (lvalue_kind) <case MODOP_EXPR>: Consider the
type of a simple assignment expression.
gcc/testsuite/ChangeLog:
* g++.dg/template/non-dependent32.C: New test.
---
gcc/cp/tree.cc | 7 +++++++
.../g++.dg/template/non-dependent32.C | 18 ++++++++++++++++++
2 files changed, 25 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/template/non-dependent32.C
Comments
On Thu, 9 May 2024, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> OK for trunk/14? For trunk as a follow-up I can implement the
> mentionted representation change to use CALL_EXPR instead of
> MODOP_EXPR for a non-dependent simple assignment expression that
> resolved to an operator= overload.
FWIW, this is the WIP patch for that including the -Wparentheses
logic adjustments needed to avoid regressing
g++.dg/warn/Wparentheses-{32,33}.C
PR c++/114994
gcc/cp/ChangeLog:
* call.cc (build_new_op): Pass 'overload' to
cp_build_modify_expr.
* cp-tree.h (cp_build_modify_expr): New overload that
takes a tree* out-parameter.
* pt.cc (tsubst_expr) <case CALL_EXPR>: Propagate
OPT_Wparentheses warning suppression to the result.
* semantics.cc (is_assignment_op_expr_p): Also recognize
templated operator expressions represented as a CALL_EXPR
to operator=.
* typeck.cc (cp_build_modify_expr): Add 'overload'
out-parameter and pass it to build_new_op.
(build_x_modify_expr): Pass 'overload' to cp_build_modify_expr.
---
gcc/cp/call.cc | 2 +-
gcc/cp/cp-tree.h | 3 +++
gcc/cp/pt.cc | 2 ++
gcc/cp/semantics.cc | 11 +++++++++++
gcc/cp/typeck.cc | 18 ++++++++++++++----
5 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 7c4ecf08c4b..1cd4992330c 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -7473,7 +7473,7 @@ build_new_op (const op_location_t &loc, enum tree_code code, int flags,
switch (code)
{
case MODIFY_EXPR:
- return cp_build_modify_expr (loc, arg1, code2, arg2, complain);
+ return cp_build_modify_expr (loc, arg1, code2, arg2, overload, complain);
case INDIRECT_REF:
return cp_build_indirect_ref (loc, arg1, RO_UNARY_STAR, complain);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f82446331b3..505c04c6e52 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8264,6 +8264,9 @@ extern tree cp_build_c_cast (location_t, tree, tree,
extern cp_expr build_x_modify_expr (location_t, tree,
enum tree_code, tree,
tree, tsubst_flags_t);
+extern tree cp_build_modify_expr (location_t, tree,
+ enum tree_code, tree,
+ tree *, tsubst_flags_t);
extern tree cp_build_modify_expr (location_t, tree,
enum tree_code, tree,
tsubst_flags_t);
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index f3d52acaaac..bc71e534cf8 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -21091,6 +21091,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
if (warning_suppressed_p (t, OPT_Wpessimizing_move))
/* This also suppresses -Wredundant-move. */
suppress_warning (ret, OPT_Wpessimizing_move);
+ if (warning_suppressed_p (t, OPT_Wparentheses))
+ suppress_warning (STRIP_REFERENCE_REF (ret), OPT_Wparentheses);
}
RETURN (ret);
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index b8c2bf8771f..e81f2b50d80 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -863,6 +863,17 @@ is_assignment_op_expr_p (tree t)
return false;
tree fndecl = cp_get_callee_fndecl_nofold (call);
+ if (!fndecl
+ && processing_template_decl
+ && TREE_CODE (CALL_EXPR_FN (call)) == COMPONENT_REF)
+ {
+ /* Also recognize (non-dependent) templated operator expressions that
+ are represented as a direct call to operator=.
+ TODO: maybe move this handling to cp_get_fndecl_from_callee for
+ benefit of other callers. */
+ if (tree fns = maybe_get_fns (TREE_OPERAND (CALL_EXPR_FN (call), 1)))
+ fndecl = OVL_FIRST (fns);
+ }
return fndecl != NULL_TREE
&& DECL_ASSIGNMENT_OPERATOR_P (fndecl)
&& DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR);
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 5f16994300f..75b696e32e0 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -9421,7 +9421,7 @@ build_modify_expr (location_t location,
tree
cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
- tree rhs, tsubst_flags_t complain)
+ tree rhs, tree *overload, tsubst_flags_t complain)
{
lhs = mark_lvalue_use_nonread (lhs);
@@ -9533,7 +9533,8 @@ cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
rhs = unshare_expr (rhs);
tree op2 = TREE_OPERAND (lhs, 2);
if (TREE_CODE (op2) != THROW_EXPR)
- op2 = cp_build_modify_expr (loc, op2, modifycode, rhs, complain);
+ op2 = cp_build_modify_expr (loc, op2, modifycode, rhs,
+ overload, complain);
tree cond = build_conditional_expr (input_location,
TREE_OPERAND (lhs, 0), op1, op2,
complain);
@@ -9620,7 +9621,7 @@ cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
result = build_new_op (input_location, MODIFY_EXPR,
LOOKUP_NORMAL, lhs, rhs,
make_node (NOP_EXPR), NULL_TREE,
- /*overload=*/NULL, complain);
+ overload, complain);
if (result == NULL_TREE)
return error_mark_node;
goto ret;
@@ -9828,6 +9829,14 @@ cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
return result;
}
+tree
+cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
+ tree rhs, tsubst_flags_t complain)
+{
+ return cp_build_modify_expr (loc, lhs, modifycode, rhs,
+ /*overload=*/nullptr, complain);
+}
+
cp_expr
build_x_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
tree rhs, tree lookups, tsubst_flags_t complain)
@@ -9856,7 +9865,8 @@ build_x_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
tree rval;
if (modifycode == NOP_EXPR)
- rval = cp_build_modify_expr (loc, lhs, modifycode, rhs, complain);
+ rval = cp_build_modify_expr (loc, lhs, modifycode, rhs,
+ &overload, complain);
else
rval = build_new_op (loc, MODIFY_EXPR, LOOKUP_NORMAL,
lhs, rhs, op, lookups, &overload, complain);
On Thu, 9 May 2024, Patrick Palka wrote:
> On Thu, 9 May 2024, Patrick Palka wrote:
>
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > OK for trunk/14? For trunk as a follow-up I can implement the
> > mentionted representation change to use CALL_EXPR instead of
> > MODOP_EXPR for a non-dependent simple assignment expression that
> > resolved to an operator= overload.
>
> FWIW, this is the WIP patch for that including the -Wparentheses
> logic adjustments needed to avoid regressing
> g++.dg/warn/Wparentheses-{32,33}.C
This patch survives bootstrap+regtest FWIW. I'm not sure which approach
we should go with for backporting.
>
> PR c++/114994
>
> gcc/cp/ChangeLog:
>
> * call.cc (build_new_op): Pass 'overload' to
> cp_build_modify_expr.
> * cp-tree.h (cp_build_modify_expr): New overload that
> takes a tree* out-parameter.
> * pt.cc (tsubst_expr) <case CALL_EXPR>: Propagate
> OPT_Wparentheses warning suppression to the result.
> * semantics.cc (is_assignment_op_expr_p): Also recognize
> templated operator expressions represented as a CALL_EXPR
> to operator=.
> * typeck.cc (cp_build_modify_expr): Add 'overload'
> out-parameter and pass it to build_new_op.
> (build_x_modify_expr): Pass 'overload' to cp_build_modify_expr.
> ---
> gcc/cp/call.cc | 2 +-
> gcc/cp/cp-tree.h | 3 +++
> gcc/cp/pt.cc | 2 ++
> gcc/cp/semantics.cc | 11 +++++++++++
> gcc/cp/typeck.cc | 18 ++++++++++++++----
> 5 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 7c4ecf08c4b..1cd4992330c 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -7473,7 +7473,7 @@ build_new_op (const op_location_t &loc, enum tree_code code, int flags,
> switch (code)
> {
> case MODIFY_EXPR:
> - return cp_build_modify_expr (loc, arg1, code2, arg2, complain);
> + return cp_build_modify_expr (loc, arg1, code2, arg2, overload, complain);
>
> case INDIRECT_REF:
> return cp_build_indirect_ref (loc, arg1, RO_UNARY_STAR, complain);
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index f82446331b3..505c04c6e52 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -8264,6 +8264,9 @@ extern tree cp_build_c_cast (location_t, tree, tree,
> extern cp_expr build_x_modify_expr (location_t, tree,
> enum tree_code, tree,
> tree, tsubst_flags_t);
> +extern tree cp_build_modify_expr (location_t, tree,
> + enum tree_code, tree,
> + tree *, tsubst_flags_t);
> extern tree cp_build_modify_expr (location_t, tree,
> enum tree_code, tree,
> tsubst_flags_t);
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index f3d52acaaac..bc71e534cf8 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -21091,6 +21091,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
> if (warning_suppressed_p (t, OPT_Wpessimizing_move))
> /* This also suppresses -Wredundant-move. */
> suppress_warning (ret, OPT_Wpessimizing_move);
> + if (warning_suppressed_p (t, OPT_Wparentheses))
> + suppress_warning (STRIP_REFERENCE_REF (ret), OPT_Wparentheses);
> }
>
> RETURN (ret);
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index b8c2bf8771f..e81f2b50d80 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -863,6 +863,17 @@ is_assignment_op_expr_p (tree t)
> return false;
>
> tree fndecl = cp_get_callee_fndecl_nofold (call);
> + if (!fndecl
> + && processing_template_decl
> + && TREE_CODE (CALL_EXPR_FN (call)) == COMPONENT_REF)
> + {
> + /* Also recognize (non-dependent) templated operator expressions that
> + are represented as a direct call to operator=.
> + TODO: maybe move this handling to cp_get_fndecl_from_callee for
> + benefit of other callers. */
> + if (tree fns = maybe_get_fns (TREE_OPERAND (CALL_EXPR_FN (call), 1)))
> + fndecl = OVL_FIRST (fns);
> + }
> return fndecl != NULL_TREE
> && DECL_ASSIGNMENT_OPERATOR_P (fndecl)
> && DECL_OVERLOADED_OPERATOR_IS (fndecl, NOP_EXPR);
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 5f16994300f..75b696e32e0 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -9421,7 +9421,7 @@ build_modify_expr (location_t location,
>
> tree
> cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
> - tree rhs, tsubst_flags_t complain)
> + tree rhs, tree *overload, tsubst_flags_t complain)
> {
> lhs = mark_lvalue_use_nonread (lhs);
>
> @@ -9533,7 +9533,8 @@ cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
> rhs = unshare_expr (rhs);
> tree op2 = TREE_OPERAND (lhs, 2);
> if (TREE_CODE (op2) != THROW_EXPR)
> - op2 = cp_build_modify_expr (loc, op2, modifycode, rhs, complain);
> + op2 = cp_build_modify_expr (loc, op2, modifycode, rhs,
> + overload, complain);
> tree cond = build_conditional_expr (input_location,
> TREE_OPERAND (lhs, 0), op1, op2,
> complain);
> @@ -9620,7 +9621,7 @@ cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
> result = build_new_op (input_location, MODIFY_EXPR,
> LOOKUP_NORMAL, lhs, rhs,
> make_node (NOP_EXPR), NULL_TREE,
> - /*overload=*/NULL, complain);
> + overload, complain);
> if (result == NULL_TREE)
> return error_mark_node;
> goto ret;
> @@ -9828,6 +9829,14 @@ cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
> return result;
> }
>
> +tree
> +cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
> + tree rhs, tsubst_flags_t complain)
> +{
> + return cp_build_modify_expr (loc, lhs, modifycode, rhs,
> + /*overload=*/nullptr, complain);
> +}
> +
> cp_expr
> build_x_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
> tree rhs, tree lookups, tsubst_flags_t complain)
> @@ -9856,7 +9865,8 @@ build_x_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
>
> tree rval;
> if (modifycode == NOP_EXPR)
> - rval = cp_build_modify_expr (loc, lhs, modifycode, rhs, complain);
> + rval = cp_build_modify_expr (loc, lhs, modifycode, rhs,
> + &overload, complain);
> else
> rval = build_new_op (loc, MODIFY_EXPR, LOOKUP_NORMAL,
> lhs, rhs, op, lookups, &overload, complain);
> --
> 2.45.0.119.g0f3415f1f8
>
>
>
> >
> > -- >8 --
> >
> > r14-4111 made us check non-dependent assignment expressions ahead of
> > time, as well as give them a type. Unlike for compound assignment
> > expressions however, if a simple assignment resolves to an operator
> > overload we still represent it as a (typed) MODOP_EXPR instead of a
> > CALL_EXPR to the selected overload. This, I reckoned, was just a
> > pessimization (since we'll have to repeat overload resolution at
> > instantiatiation time) but should be harmless. (And it should be
> > easily fixable by giving cp_build_modify_expr an 'overload' parameter).
> >
> > But it breaks the below testcase ultimately because MODOP_EXPR (of
> > non-reference type) is always treated as an lvalue according to
> > lvalue_kind, which is incorrect for the MODOP_EXPR representing x=42.
> >
> > We can fix this by representing such assignment expressions as CALL_EXPRs
> > matching what that of compound assignments, but that turns out to
> > require some tweaking of our -Wparentheses warning logic which seems
> > unsuitable for backporting.
> >
> > So this patch instead more conservatively fixes this by refining
> > lvalue_kind to consider the type of a (simple) MODOP_EXPR as we
> > already do for COND_EXPR.
> >
> > PR c++/114994
> >
> > gcc/cp/ChangeLog:
> >
> > * tree.cc (lvalue_kind) <case MODOP_EXPR>: Consider the
> > type of a simple assignment expression.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/template/non-dependent32.C: New test.
> > ---
> > gcc/cp/tree.cc | 7 +++++++
> > .../g++.dg/template/non-dependent32.C | 18 ++++++++++++++++++
> > 2 files changed, 25 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/template/non-dependent32.C
> >
> > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> > index f1a23ffe817..0b97b789aab 100644
> > --- a/gcc/cp/tree.cc
> > +++ b/gcc/cp/tree.cc
> > @@ -275,6 +275,13 @@ lvalue_kind (const_tree ref)
> > /* We expect to see unlowered MODOP_EXPRs only during
> > template processing. */
> > gcc_assert (processing_template_decl);
> > + if (TREE_CODE (TREE_OPERAND (ref, 1)) == NOP_EXPR
> > + && CLASS_TYPE_P (TREE_TYPE (TREE_OPERAND (ref, 0))))
> > + /* As in the COND_EXPR case, but for non-dependent assignment
> > + expressions created by build_x_modify_expr. */
> > + goto default_;
> > + /* A non-dependent (simple or compound) assignment expression that
> > + resolved to a built-in assignment function. */
> > return clk_ordinary;
Note that the reason we can't use the default_ case unconditionally is
because assignments to a scalar have non-reference type, so the
default_ case would incorrectly return clk_none instead of clk_ordinary
for them (since they would not get handled by the TYPE_REF_P case near
the beginning of lvalue_kind).
Assignments to a class however seem to alway resolve to a (perhaps
implicit) operator=, so they will have reference type (if operator=
returns a reference) and get handled by the TYPE_REF_P case as
appropriate and otherwise we can safely fall back to the default_ case.
And note that non-dependent compound assignment expressions don't
exhibit this bug because for class types we represent them as CALL_EXPR
to the selected operator= overload, which lvalue_kind naturally handles
correctly.
> >
> > case MODIFY_EXPR:
> > diff --git a/gcc/testsuite/g++.dg/template/non-dependent32.C b/gcc/testsuite/g++.dg/template/non-dependent32.C
> > new file mode 100644
> > index 00000000000..54252c7dfaf
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/non-dependent32.C
> > @@ -0,0 +1,18 @@
> > +// PR c++/114994
> > +// { dg-do compile { target c++11 } }
> > +
> > +struct udl_arg {
> > + udl_arg operator=(int);
> > +};
> > +
> > +void f(udl_arg&&);
> > +
> > +template<class>
> > +void g() {
> > + udl_arg x;
> > + f(x=42); // { dg-bogus "cannot bind" }
> > +}
> > +
> > +int main() {
> > + g<int>();
> > +}
> > --
> > 2.45.0.119.g0f3415f1f8
> >
> >
>
On 5/9/24 16:23, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> OK for trunk/14? For trunk as a follow-up I can implement the
> mentionted representation change to use CALL_EXPR instead of
> MODOP_EXPR for a non-dependent simple assignment expression that
> resolved to an operator= overload.
>
> -- >8 --
>
> r14-4111 made us check non-dependent assignment expressions ahead of
> time, as well as give them a type. Unlike for compound assignment
> expressions however, if a simple assignment resolves to an operator
> overload we still represent it as a (typed) MODOP_EXPR instead of a
> CALL_EXPR to the selected overload. This, I reckoned, was just a
> pessimization (since we'll have to repeat overload resolution at
> instantiatiation time) but should be harmless. (And it should be
> easily fixable by giving cp_build_modify_expr an 'overload' parameter).
>
> But it breaks the below testcase ultimately because MODOP_EXPR (of
> non-reference type) is always treated as an lvalue according to
> lvalue_kind, which is incorrect for the MODOP_EXPR representing x=42.
>
> We can fix this by representing such assignment expressions as CALL_EXPRs
> matching what that of compound assignments, but that turns out to
> require some tweaking of our -Wparentheses warning logic which seems
> unsuitable for backporting.
>
> So this patch instead more conservatively fixes this by refining
> lvalue_kind to consider the type of a (simple) MODOP_EXPR as we
> already do for COND_EXPR.
>
> PR c++/114994
>
> gcc/cp/ChangeLog:
>
> * tree.cc (lvalue_kind) <case MODOP_EXPR>: Consider the
> type of a simple assignment expression.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/template/non-dependent32.C: New test.
> ---
> gcc/cp/tree.cc | 7 +++++++
> .../g++.dg/template/non-dependent32.C | 18 ++++++++++++++++++
> 2 files changed, 25 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/template/non-dependent32.C
>
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index f1a23ffe817..0b97b789aab 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -275,6 +275,13 @@ lvalue_kind (const_tree ref)
> /* We expect to see unlowered MODOP_EXPRs only during
> template processing. */
> gcc_assert (processing_template_decl);
> + if (TREE_CODE (TREE_OPERAND (ref, 1)) == NOP_EXPR
> + && CLASS_TYPE_P (TREE_TYPE (TREE_OPERAND (ref, 0))))
> + /* As in the COND_EXPR case, but for non-dependent assignment
> + expressions created by build_x_modify_expr. */
> + goto default_;
This seems overly specific, I'd think the same thing would apply to +=
and such?
Jason
On 5/9/24 16:29, Patrick Palka wrote:
> On Thu, 9 May 2024, Patrick Palka wrote:
>
>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
>> OK for trunk/14? For trunk as a follow-up I can implement the
>> mentionted representation change to use CALL_EXPR instead of
>> MODOP_EXPR for a non-dependent simple assignment expression that
>> resolved to an operator= overload.
>
> FWIW, this is the WIP patch for that including the -Wparentheses
> logic adjustments needed to avoid regressing
> g++.dg/warn/Wparentheses-{32,33}.C
>
> PR c++/114994
>
> gcc/cp/ChangeLog:
>
> * call.cc (build_new_op): Pass 'overload' to
> cp_build_modify_expr.
> * cp-tree.h (cp_build_modify_expr): New overload that
> takes a tree* out-parameter.
> * pt.cc (tsubst_expr) <case CALL_EXPR>: Propagate
> OPT_Wparentheses warning suppression to the result.
> * semantics.cc (is_assignment_op_expr_p): Also recognize
> templated operator expressions represented as a CALL_EXPR
> to operator=.
> * typeck.cc (cp_build_modify_expr): Add 'overload'
> out-parameter and pass it to build_new_op.
> (build_x_modify_expr): Pass 'overload' to cp_build_modify_expr.
> ---
> gcc/cp/call.cc | 2 +-
> gcc/cp/cp-tree.h | 3 +++
> gcc/cp/pt.cc | 2 ++
> gcc/cp/semantics.cc | 11 +++++++++++
> gcc/cp/typeck.cc | 18 ++++++++++++++----
> 5 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 7c4ecf08c4b..1cd4992330c 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -7473,7 +7473,7 @@ build_new_op (const op_location_t &loc, enum tree_code code, int flags,
> switch (code)
> {
> case MODIFY_EXPR:
> - return cp_build_modify_expr (loc, arg1, code2, arg2, complain);
> + return cp_build_modify_expr (loc, arg1, code2, arg2, overload, complain);
>
> case INDIRECT_REF:
> return cp_build_indirect_ref (loc, arg1, RO_UNARY_STAR, complain);
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index f82446331b3..505c04c6e52 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -8264,6 +8264,9 @@ extern tree cp_build_c_cast (location_t, tree, tree,
> extern cp_expr build_x_modify_expr (location_t, tree,
> enum tree_code, tree,
> tree, tsubst_flags_t);
> +extern tree cp_build_modify_expr (location_t, tree,
> + enum tree_code, tree,
> + tree *, tsubst_flags_t);
> extern tree cp_build_modify_expr (location_t, tree,
> enum tree_code, tree,
> tsubst_flags_t);
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index f3d52acaaac..bc71e534cf8 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -21091,6 +21091,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
> if (warning_suppressed_p (t, OPT_Wpessimizing_move))
> /* This also suppresses -Wredundant-move. */
> suppress_warning (ret, OPT_Wpessimizing_move);
> + if (warning_suppressed_p (t, OPT_Wparentheses))
> + suppress_warning (STRIP_REFERENCE_REF (ret), OPT_Wparentheses);
> }
>
> RETURN (ret);
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index b8c2bf8771f..e81f2b50d80 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -863,6 +863,17 @@ is_assignment_op_expr_p (tree t)
> return false;
>
> tree fndecl = cp_get_callee_fndecl_nofold (call);
> + if (!fndecl
> + && processing_template_decl
> + && TREE_CODE (CALL_EXPR_FN (call)) == COMPONENT_REF)
> + {
> + /* Also recognize (non-dependent) templated operator expressions that
> + are represented as a direct call to operator=.
> + TODO: maybe move this handling to cp_get_fndecl_from_callee for
> + benefit of other callers. */
Please.
Jason
On Fri, 10 May 2024, Jason Merrill wrote:
> On 5/9/24 16:23, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > OK for trunk/14? For trunk as a follow-up I can implement the
> > mentionted representation change to use CALL_EXPR instead of
> > MODOP_EXPR for a non-dependent simple assignment expression that
> > resolved to an operator= overload.
> >
> > -- >8 --
> >
> > r14-4111 made us check non-dependent assignment expressions ahead of
> > time, as well as give them a type. Unlike for compound assignment
> > expressions however, if a simple assignment resolves to an operator
> > overload we still represent it as a (typed) MODOP_EXPR instead of a
> > CALL_EXPR to the selected overload. This, I reckoned, was just a
> > pessimization (since we'll have to repeat overload resolution at
> > instantiatiation time) but should be harmless. (And it should be
> > easily fixable by giving cp_build_modify_expr an 'overload' parameter).
> >
> > But it breaks the below testcase ultimately because MODOP_EXPR (of
> > non-reference type) is always treated as an lvalue according to
> > lvalue_kind, which is incorrect for the MODOP_EXPR representing x=42.
> >
> > We can fix this by representing such assignment expressions as CALL_EXPRs
> > matching what that of compound assignments, but that turns out to
> > require some tweaking of our -Wparentheses warning logic which seems
> > unsuitable for backporting.
> >
> > So this patch instead more conservatively fixes this by refining
> > lvalue_kind to consider the type of a (simple) MODOP_EXPR as we
> > already do for COND_EXPR.
> >
> > PR c++/114994
> >
> > gcc/cp/ChangeLog:
> >
> > * tree.cc (lvalue_kind) <case MODOP_EXPR>: Consider the
> > type of a simple assignment expression.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/template/non-dependent32.C: New test.
> > ---
> > gcc/cp/tree.cc | 7 +++++++
> > .../g++.dg/template/non-dependent32.C | 18 ++++++++++++++++++
> > 2 files changed, 25 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/template/non-dependent32.C
> >
> > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> > index f1a23ffe817..0b97b789aab 100644
> > --- a/gcc/cp/tree.cc
> > +++ b/gcc/cp/tree.cc
> > @@ -275,6 +275,13 @@ lvalue_kind (const_tree ref)
> > /* We expect to see unlowered MODOP_EXPRs only during
> > template processing. */
> > gcc_assert (processing_template_decl);
> > + if (TREE_CODE (TREE_OPERAND (ref, 1)) == NOP_EXPR
> > + && CLASS_TYPE_P (TREE_TYPE (TREE_OPERAND (ref, 0))))
> > + /* As in the COND_EXPR case, but for non-dependent assignment
> > + expressions created by build_x_modify_expr. */
> > + goto default_;
>
> This seems overly specific, I'd think the same thing would apply to += and
> such?
We shouldn't see += etc of class type here since we already represent
those as CALL_EXPR to the selected operator=, but indeed it could
otherwise apply to +=. Like so?
-- >8 --
Subject: [PATCH] c++: lvalueness of non-dependent assignment expr [PR114994]
PR c++/114994
gcc/cp/ChangeLog:
* tree.cc (lvalue_kind) <case MODOP_EXPR>: Consider the
type of a class assignment expression.
gcc/testsuite/ChangeLog:
* g++.dg/template/non-dependent32.C: New test.
---
gcc/cp/tree.cc | 5 ++++-
.../g++.dg/template/non-dependent32.C | 18 ++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/template/non-dependent32.C
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index f1a23ffe817..9d37d255d8d 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -275,7 +275,10 @@ lvalue_kind (const_tree ref)
/* We expect to see unlowered MODOP_EXPRs only during
template processing. */
gcc_assert (processing_template_decl);
- return clk_ordinary;
+ if (CLASS_TYPE_P (TREE_TYPE (TREE_OPERAND (ref, 0))))
+ goto default_;
+ else
+ return clk_ordinary;
case MODIFY_EXPR:
case TYPEID_EXPR:
diff --git a/gcc/testsuite/g++.dg/template/non-dependent32.C b/gcc/testsuite/g++.dg/template/non-dependent32.C
new file mode 100644
index 00000000000..54252c7dfaf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/non-dependent32.C
@@ -0,0 +1,18 @@
+// PR c++/114994
+// { dg-do compile { target c++11 } }
+
+struct udl_arg {
+ udl_arg operator=(int);
+};
+
+void f(udl_arg&&);
+
+template<class>
+void g() {
+ udl_arg x;
+ f(x=42); // { dg-bogus "cannot bind" }
+}
+
+int main() {
+ g<int>();
+}
On 5/11/24 20:46, Patrick Palka wrote:
> On Fri, 10 May 2024, Jason Merrill wrote:
>
>> On 5/9/24 16:23, Patrick Palka wrote:
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
>>> OK for trunk/14? For trunk as a follow-up I can implement the
>>> mentionted representation change to use CALL_EXPR instead of
>>> MODOP_EXPR for a non-dependent simple assignment expression that
>>> resolved to an operator= overload.
>>>
>>> -- >8 --
>>>
>>> r14-4111 made us check non-dependent assignment expressions ahead of
>>> time, as well as give them a type. Unlike for compound assignment
>>> expressions however, if a simple assignment resolves to an operator
>>> overload we still represent it as a (typed) MODOP_EXPR instead of a
>>> CALL_EXPR to the selected overload. This, I reckoned, was just a
>>> pessimization (since we'll have to repeat overload resolution at
>>> instantiatiation time) but should be harmless. (And it should be
>>> easily fixable by giving cp_build_modify_expr an 'overload' parameter).
>>>
>>> But it breaks the below testcase ultimately because MODOP_EXPR (of
>>> non-reference type) is always treated as an lvalue according to
>>> lvalue_kind, which is incorrect for the MODOP_EXPR representing x=42.
>>>
>>> We can fix this by representing such assignment expressions as CALL_EXPRs
>>> matching what that of compound assignments, but that turns out to
>>> require some tweaking of our -Wparentheses warning logic which seems
>>> unsuitable for backporting.
>>>
>>> So this patch instead more conservatively fixes this by refining
>>> lvalue_kind to consider the type of a (simple) MODOP_EXPR as we
>>> already do for COND_EXPR.
>>>
>>> PR c++/114994
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> * tree.cc (lvalue_kind) <case MODOP_EXPR>: Consider the
>>> type of a simple assignment expression.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * g++.dg/template/non-dependent32.C: New test.
>>> ---
>>> gcc/cp/tree.cc | 7 +++++++
>>> .../g++.dg/template/non-dependent32.C | 18 ++++++++++++++++++
>>> 2 files changed, 25 insertions(+)
>>> create mode 100644 gcc/testsuite/g++.dg/template/non-dependent32.C
>>>
>>> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
>>> index f1a23ffe817..0b97b789aab 100644
>>> --- a/gcc/cp/tree.cc
>>> +++ b/gcc/cp/tree.cc
>>> @@ -275,6 +275,13 @@ lvalue_kind (const_tree ref)
>>> /* We expect to see unlowered MODOP_EXPRs only during
>>> template processing. */
>>> gcc_assert (processing_template_decl);
>>> + if (TREE_CODE (TREE_OPERAND (ref, 1)) == NOP_EXPR
>>> + && CLASS_TYPE_P (TREE_TYPE (TREE_OPERAND (ref, 0))))
>>> + /* As in the COND_EXPR case, but for non-dependent assignment
>>> + expressions created by build_x_modify_expr. */
>>> + goto default_;
>>
>> This seems overly specific, I'd think the same thing would apply to += and
>> such?
>
> We shouldn't see += etc of class type here since we already represent
> those as CALL_EXPR to the selected operator=, but indeed it could
> otherwise apply to +=. Like so?
OK.
> -- >8 --
>
> Subject: [PATCH] c++: lvalueness of non-dependent assignment expr [PR114994]
>
> PR c++/114994
>
> gcc/cp/ChangeLog:
>
> * tree.cc (lvalue_kind) <case MODOP_EXPR>: Consider the
> type of a class assignment expression.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/template/non-dependent32.C: New test.
> ---
> gcc/cp/tree.cc | 5 ++++-
> .../g++.dg/template/non-dependent32.C | 18 ++++++++++++++++++
> 2 files changed, 22 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/template/non-dependent32.C
>
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index f1a23ffe817..9d37d255d8d 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -275,7 +275,10 @@ lvalue_kind (const_tree ref)
> /* We expect to see unlowered MODOP_EXPRs only during
> template processing. */
> gcc_assert (processing_template_decl);
> - return clk_ordinary;
> + if (CLASS_TYPE_P (TREE_TYPE (TREE_OPERAND (ref, 0))))
> + goto default_;
> + else
> + return clk_ordinary;
>
> case MODIFY_EXPR:
> case TYPEID_EXPR:
> diff --git a/gcc/testsuite/g++.dg/template/non-dependent32.C b/gcc/testsuite/g++.dg/template/non-dependent32.C
> new file mode 100644
> index 00000000000..54252c7dfaf
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/non-dependent32.C
> @@ -0,0 +1,18 @@
> +// PR c++/114994
> +// { dg-do compile { target c++11 } }
> +
> +struct udl_arg {
> + udl_arg operator=(int);
> +};
> +
> +void f(udl_arg&&);
> +
> +template<class>
> +void g() {
> + udl_arg x;
> + f(x=42); // { dg-bogus "cannot bind" }
> +}
> +
> +int main() {
> + g<int>();
> +}
@@ -275,6 +275,13 @@ lvalue_kind (const_tree ref)
/* We expect to see unlowered MODOP_EXPRs only during
template processing. */
gcc_assert (processing_template_decl);
+ if (TREE_CODE (TREE_OPERAND (ref, 1)) == NOP_EXPR
+ && CLASS_TYPE_P (TREE_TYPE (TREE_OPERAND (ref, 0))))
+ /* As in the COND_EXPR case, but for non-dependent assignment
+ expressions created by build_x_modify_expr. */
+ goto default_;
+ /* A non-dependent (simple or compound) assignment expression that
+ resolved to a built-in assignment function. */
return clk_ordinary;
case MODIFY_EXPR:
new file mode 100644
@@ -0,0 +1,18 @@
+// PR c++/114994
+// { dg-do compile { target c++11 } }
+
+struct udl_arg {
+ udl_arg operator=(int);
+};
+
+void f(udl_arg&&);
+
+template<class>
+void g() {
+ udl_arg x;
+ f(x=42); // { dg-bogus "cannot bind" }
+}
+
+int main() {
+ g<int>();
+}