Re: [PATCH] c++: lvalueness of non-dependent assignment [PR114994]

2024-05-14 Thread Jason Merrill

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) : 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) : 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 000..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
+void g() {
+  udl_arg x;
+  f(x=42); // { dg-bogus "cannot bind" }
+}
+
+int main() {
+  g();
+}




Re: [PATCH] c++: lvalueness of non-dependent assignment [PR114994]

2024-05-11 Thread Patrick Palka
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) : 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) : 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 000..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
+void g() {
+  udl_arg x;
+  f(x=42); // { dg-bogus "cannot bind" }
+}
+
+int main() {
+  g();
+}
-- 
2.45.0.119.g0f3415f1f8


... and here's the updated patch to make us represent simple class
assignment as CALL_EXPR as well (should the first ptach go in for 14/trunk,
and this second patch for trunk only?):

Subject: [PATCH] c++: lvalueness of non-dependent assignment expr [PR114994]

PR c++/114994

gcc/cp/ChangeLog:

* call.cc (build_new_op): Pass 'overload' to
cp_

Re: [PATCH] c++: lvalueness of non-dependent assignment [PR114994]

2024-05-10 Thread Jason Merrill

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) : 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



Re: [PATCH] c++: lvalueness of non-dependent assignment [PR114994]

2024-05-10 Thread Jason Merrill

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) : 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



Re: [PATCH] c++: lvalueness of non-dependent assignment [PR114994]

2024-05-10 Thread Patrick Palka
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) : 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)
> +   

Re: [PATCH] c++: lvalueness of non-dependent assignment [PR114994]

2024-05-09 Thread Patrick Palka
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) : 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 (op

[PATCH] c++: lvalueness of non-dependent assignment [PR114994]

2024-05-09 Thread Patrick Palka
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) : 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;
 
 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 000..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
+void g() {
+  udl_arg x;
+  f(x=42); // { dg-bogus "cannot bind" }
+}
+
+int main() {
+  g();
+}
-- 
2.45.0.119.g0f3415f1f8