| Message ID | 20240509202300.2742125-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: lvalueness of non-dependent assignment [PR114994] | expand |
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>(); > +}
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; 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>(); +}