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

[Patrick Palka <ppalka@redhat.com>]

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