public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: dependent operator expression lookup [PR51577]
@ 2021-05-10  2:33 Patrick Palka
  2021-05-10 14:51 ` Jason Merrill
  0 siblings, 1 reply; 2+ messages in thread
From: Patrick Palka @ 2021-05-10  2:33 UTC (permalink / raw)
  To: gcc-patches

This unconditionally enables the maybe_save_operator_binding mechanism
for all function templates, so that when resolving a dependent operator
expression from a function template we ignore later-declared
namespace-scope bindings that weren't visible at template definition
time.  This patch additionally makes the mechanism apply to dependent
comma and compound-assignment operator expressions.

Note that this doesn't fix the testcases in PR83035 or PR99692 because
there the dependent operator expressions aren't at function scope.  I'm
not sure how exactly to fix these testcases using the current approach,
since although we'll in both testcases have a TEMPLATE_DECL to associate
the lookup result with, at instantiation time we won't have an
appropriate binding level to push to.  I wonder we could instead encode
dependent operator expressions as appropriately-flagged CALL_EXPRs?

Bootstrapped and regtested on x86_64-pc-linux-gnu, and tested on cmcstl2
and range-v3 and numerous boost libraries, does this look OK for trunk?

gcc/cp/ChangeLog:

	PR c++/51577
	* name-lookup.c (maybe_save_operator_binding): Unconditionally
	enable for all function templates, not just generic lambdas.
	* typeck.c (build_x_compound_expr): Call maybe_save_operator_binding
	in the type-dependent case.
	(build_x_modify_expr): Likewise.  Move declaration of 'op'
	closer to its first use.

gcc/testsuite/ChangeLog:

	PR c++/51577
	* g++.dg/lookup/operator-3.C: New test.
---
 gcc/cp/name-lookup.c                     |  15 ++--
 gcc/cp/typeck.c                          |  17 ++--
 gcc/testsuite/g++.dg/lookup/operator-3.C | 109 +++++++++++++++++++++++
 3 files changed, 128 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/lookup/operator-3.C

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 4e84e2f9987..a6c9e68a19e 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -9116,7 +9116,7 @@ static const char *const op_bind_attrname = "operator bindings";
 void
 maybe_save_operator_binding (tree e)
 {
-  /* This is only useful in a generic lambda.  */
+  /* This is only useful in a template.  */
   if (!processing_template_decl)
     return;
 
@@ -9124,13 +9124,12 @@ maybe_save_operator_binding (tree e)
   if (!cfn)
     return;
 
-  /* Do this for lambdas and code that will emit a CMI.  In a module's
-     GMF we don't yet know whether there will be a CMI.  */
-  if (!module_has_cmi_p () && !global_purview_p () && !current_lambda_expr())
-     return;
-
-  tree fnname = ovl_op_identifier (false, TREE_CODE (e));
-  if (!fnname)
+  tree fnname;
+  if(TREE_CODE (e) == MODOP_EXPR)
+    fnname = ovl_op_identifier (true, TREE_CODE (TREE_OPERAND (e, 1)));
+  else
+    fnname = ovl_op_identifier (false, TREE_CODE (e));
+  if (!fnname || fnname == assign_op_identifier)
     return;
 
   tree attributes = DECL_ATTRIBUTES (cfn);
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 50d0f1e6a62..6826fcf139c 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -7272,7 +7272,11 @@ build_x_compound_expr (location_t loc, tree op1, tree op2,
     {
       if (type_dependent_expression_p (op1)
 	  || type_dependent_expression_p (op2))
-	return build_min_nt_loc (loc, COMPOUND_EXPR, op1, op2);
+	{
+	  result = build_min_nt_loc (loc, COMPOUND_EXPR, op1, op2);
+	  maybe_save_operator_binding (result);
+	  return result;
+	}
       op1 = build_non_dependent_expr (op1);
       op2 = build_non_dependent_expr (op2);
     }
@@ -8936,7 +8940,6 @@ build_x_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
   tree orig_lhs = lhs;
   tree orig_rhs = rhs;
   tree overload = NULL_TREE;
-  tree op = build_nt (modifycode, NULL_TREE, NULL_TREE);
 
   if (lhs == error_mark_node || rhs == error_mark_node)
     return cp_expr (error_mark_node, loc);
@@ -8946,9 +8949,12 @@ build_x_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
       if (modifycode == NOP_EXPR
 	  || type_dependent_expression_p (lhs)
 	  || type_dependent_expression_p (rhs))
-        return build_min_nt_loc (loc, MODOP_EXPR, lhs,
-				 build_min_nt_loc (loc, modifycode, NULL_TREE,
-						   NULL_TREE), rhs);
+	{
+	  tree op = build_min_nt_loc (loc, modifycode, NULL_TREE, NULL_TREE);
+	  tree rval = build_min_nt_loc (loc, MODOP_EXPR, lhs, op, rhs);
+	  maybe_save_operator_binding (rval);
+	  return rval;
+	}
 
       lhs = build_non_dependent_expr (lhs);
       rhs = build_non_dependent_expr (rhs);
@@ -8956,6 +8962,7 @@ build_x_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
 
   if (modifycode != NOP_EXPR)
     {
+      tree op = build_nt (modifycode, NULL_TREE, NULL_TREE);
       tree rval = build_new_op (loc, MODIFY_EXPR, LOOKUP_NORMAL,
 				lhs, rhs, op, &overload, complain);
       if (rval)
diff --git a/gcc/testsuite/g++.dg/lookup/operator-3.C b/gcc/testsuite/g++.dg/lookup/operator-3.C
new file mode 100644
index 00000000000..bc5eb3d6693
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/operator-3.C
@@ -0,0 +1,109 @@
+// PR c++/51577
+
+template <class T> void f (T x) {
+  +x; // { dg-error "no match" }
+  -x; // { dg-error "no match" }
+  *x; // { dg-error "no match" }
+  ~x; // { dg-error "no match" }
+  &x;
+  !x; // { dg-error "no match" }
+  ++x; // { dg-error "no match" }
+  --x; // { dg-error "no match" }
+  x++; // { dg-error "declared for postfix" }
+  x--; // { dg-error "declared for postfix" }
+
+  x->*x; // { dg-error "no match" }
+  x / x; // { dg-error "no match" }
+  x * x; // { dg-error "no match" }
+  x + x; // { dg-error "no match" }
+  x - x; // { dg-error "no match" }
+  x % x; // { dg-error "no match" }
+  x & x; // { dg-error "no match" }
+  x | x; // { dg-error "no match" }
+  x ^ x; // { dg-error "no match" }
+  x << x; // { dg-error "no match" }
+  x >> x; // { dg-error "no match" }
+  x && x; // { dg-error "no match" }
+  x || x; // { dg-error "no match" }
+  x, x;
+
+  x == x; // { dg-error "no match" }
+  x != x; // { dg-error "no match" }
+  x < x; // { dg-error "no match" }
+  x > x; // { dg-error "no match" }
+  x <= x; // { dg-error "no match" }
+  x >= x; // { dg-error "no match" }
+#if __cplusplus > 201703L
+  x <=> x; // { dg-error "no match" "" { target c++20 } }
+#endif
+
+  x += x; // { dg-error "no match" }
+  x -= x; // { dg-error "no match" }
+  x *= x; // { dg-error "no match" }
+  x /= x; // { dg-error "no match" }
+  x %= x; // { dg-error "no match" }
+  x |= x; // { dg-error "no match" }
+  x ^= x; // { dg-error "no match" }
+  x <<= x; // { dg-error "no match" }
+  x >>= x; // { dg-error "no match" }
+}
+
+namespace N { struct A { }; }
+
+void operator+(N::A);
+void operator-(N::A);
+void operator*(N::A);
+void operator~(N::A);
+#if __cplusplus >= 201103L
+void operator&(N::A) = delete;
+#else
+void operator&(N::A);
+#endif
+void operator!(N::A);
+void operator++(N::A);
+void operator--(N::A);
+void operator++(N::A, int);
+void operator--(N::A, int);
+
+void operator->*(N::A, N::A);
+void operator/(N::A, N::A);
+void operator*(N::A, N::A);
+void operator+(N::A, N::A);
+void operator-(N::A, N::A);
+void operator%(N::A, N::A);
+void operator&(N::A, N::A);
+void operator|(N::A, N::A);
+void operator^(N::A, N::A);
+void operator<<(N::A, N::A);
+void operator>>(N::A, N::A);
+void operator&&(N::A, N::A);
+void operator||(N::A, N::A);
+#if __cplusplus >= 201103L
+void operator,(N::A, N::A) = delete;
+#else
+void operator,(N::A, N::A);
+#endif
+
+void operator==(N::A, N::A);
+void operator!=(N::A, N::A);
+void operator<(N::A, N::A);
+void operator>(N::A, N::A);
+void operator<=(N::A, N::A);
+void operator>=(N::A, N::A);
+#if __cplusplus > 201703L
+void operator<=>(N::A, N::A);
+#endif
+
+void operator+=(N::A, N::A);
+void operator-=(N::A, N::A);
+void operator*=(N::A, N::A);
+void operator/=(N::A, N::A);
+void operator%=(N::A, N::A);
+void operator|=(N::A, N::A);
+void operator^=(N::A, N::A);
+void operator<<=(N::A, N::A);
+void operator>>=(N::A, N::A);
+
+int main() {
+  f(N::A());
+}
-- 
2.31.1.527.g2d677e5b15


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] c++: dependent operator expression lookup [PR51577]
  2021-05-10  2:33 [PATCH] c++: dependent operator expression lookup [PR51577] Patrick Palka
@ 2021-05-10 14:51 ` Jason Merrill
  0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2021-05-10 14:51 UTC (permalink / raw)
  To: Patrick Palka, gcc-patches

On 5/9/21 10:33 PM, Patrick Palka wrote:
> This unconditionally enables the maybe_save_operator_binding mechanism
> for all function templates, so that when resolving a dependent operator
> expression from a function template we ignore later-declared
> namespace-scope bindings that weren't visible at template definition
> time.  This patch additionally makes the mechanism apply to dependent
> comma and compound-assignment operator expressions.
> 
> Note that this doesn't fix the testcases in PR83035 or PR99692 because
> there the dependent operator expressions aren't at function scope.  I'm
> not sure how exactly to fix these testcases using the current approach,
> since although we'll in both testcases have a TEMPLATE_DECL to associate
> the lookup result with, at instantiation time we won't have an
> appropriate binding level to push to.  I wonder we could instead encode
> dependent operator expressions as appropriately-flagged CALL_EXPRs?

That sounds plausible.

> Bootstrapped and regtested on x86_64-pc-linux-gnu, and tested on cmcstl2
> and range-v3 and numerous boost libraries, does this look OK for trunk?

OK.

> gcc/cp/ChangeLog:
> 
> 	PR c++/51577
> 	* name-lookup.c (maybe_save_operator_binding): Unconditionally
> 	enable for all function templates, not just generic lambdas.
> 	* typeck.c (build_x_compound_expr): Call maybe_save_operator_binding
> 	in the type-dependent case.
> 	(build_x_modify_expr): Likewise.  Move declaration of 'op'
> 	closer to its first use.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/51577
> 	* g++.dg/lookup/operator-3.C: New test.
> ---
>   gcc/cp/name-lookup.c                     |  15 ++--
>   gcc/cp/typeck.c                          |  17 ++--
>   gcc/testsuite/g++.dg/lookup/operator-3.C | 109 +++++++++++++++++++++++
>   3 files changed, 128 insertions(+), 13 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/lookup/operator-3.C
> 
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 4e84e2f9987..a6c9e68a19e 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -9116,7 +9116,7 @@ static const char *const op_bind_attrname = "operator bindings";
>   void
>   maybe_save_operator_binding (tree e)
>   {
> -  /* This is only useful in a generic lambda.  */
> +  /* This is only useful in a template.  */
>     if (!processing_template_decl)
>       return;
>   
> @@ -9124,13 +9124,12 @@ maybe_save_operator_binding (tree e)
>     if (!cfn)
>       return;
>   
> -  /* Do this for lambdas and code that will emit a CMI.  In a module's
> -     GMF we don't yet know whether there will be a CMI.  */
> -  if (!module_has_cmi_p () && !global_purview_p () && !current_lambda_expr())
> -     return;
> -
> -  tree fnname = ovl_op_identifier (false, TREE_CODE (e));
> -  if (!fnname)
> +  tree fnname;
> +  if(TREE_CODE (e) == MODOP_EXPR)
> +    fnname = ovl_op_identifier (true, TREE_CODE (TREE_OPERAND (e, 1)));
> +  else
> +    fnname = ovl_op_identifier (false, TREE_CODE (e));
> +  if (!fnname || fnname == assign_op_identifier)
>       return;
>   
>     tree attributes = DECL_ATTRIBUTES (cfn);
> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index 50d0f1e6a62..6826fcf139c 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -7272,7 +7272,11 @@ build_x_compound_expr (location_t loc, tree op1, tree op2,
>       {
>         if (type_dependent_expression_p (op1)
>   	  || type_dependent_expression_p (op2))
> -	return build_min_nt_loc (loc, COMPOUND_EXPR, op1, op2);
> +	{
> +	  result = build_min_nt_loc (loc, COMPOUND_EXPR, op1, op2);
> +	  maybe_save_operator_binding (result);
> +	  return result;
> +	}
>         op1 = build_non_dependent_expr (op1);
>         op2 = build_non_dependent_expr (op2);
>       }
> @@ -8936,7 +8940,6 @@ build_x_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
>     tree orig_lhs = lhs;
>     tree orig_rhs = rhs;
>     tree overload = NULL_TREE;
> -  tree op = build_nt (modifycode, NULL_TREE, NULL_TREE);
>   
>     if (lhs == error_mark_node || rhs == error_mark_node)
>       return cp_expr (error_mark_node, loc);
> @@ -8946,9 +8949,12 @@ build_x_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
>         if (modifycode == NOP_EXPR
>   	  || type_dependent_expression_p (lhs)
>   	  || type_dependent_expression_p (rhs))
> -        return build_min_nt_loc (loc, MODOP_EXPR, lhs,
> -				 build_min_nt_loc (loc, modifycode, NULL_TREE,
> -						   NULL_TREE), rhs);
> +	{
> +	  tree op = build_min_nt_loc (loc, modifycode, NULL_TREE, NULL_TREE);
> +	  tree rval = build_min_nt_loc (loc, MODOP_EXPR, lhs, op, rhs);
> +	  maybe_save_operator_binding (rval);
> +	  return rval;
> +	}
>   
>         lhs = build_non_dependent_expr (lhs);
>         rhs = build_non_dependent_expr (rhs);
> @@ -8956,6 +8962,7 @@ build_x_modify_expr (location_t loc, tree lhs, enum tree_code modifycode,
>   
>     if (modifycode != NOP_EXPR)
>       {
> +      tree op = build_nt (modifycode, NULL_TREE, NULL_TREE);
>         tree rval = build_new_op (loc, MODIFY_EXPR, LOOKUP_NORMAL,
>   				lhs, rhs, op, &overload, complain);
>         if (rval)
> diff --git a/gcc/testsuite/g++.dg/lookup/operator-3.C b/gcc/testsuite/g++.dg/lookup/operator-3.C
> new file mode 100644
> index 00000000000..bc5eb3d6693
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lookup/operator-3.C
> @@ -0,0 +1,109 @@
> +// PR c++/51577
> +
> +template <class T> void f (T x) {
> +  +x; // { dg-error "no match" }
> +  -x; // { dg-error "no match" }
> +  *x; // { dg-error "no match" }
> +  ~x; // { dg-error "no match" }
> +  &x;
> +  !x; // { dg-error "no match" }
> +  ++x; // { dg-error "no match" }
> +  --x; // { dg-error "no match" }
> +  x++; // { dg-error "declared for postfix" }
> +  x--; // { dg-error "declared for postfix" }
> +
> +  x->*x; // { dg-error "no match" }
> +  x / x; // { dg-error "no match" }
> +  x * x; // { dg-error "no match" }
> +  x + x; // { dg-error "no match" }
> +  x - x; // { dg-error "no match" }
> +  x % x; // { dg-error "no match" }
> +  x & x; // { dg-error "no match" }
> +  x | x; // { dg-error "no match" }
> +  x ^ x; // { dg-error "no match" }
> +  x << x; // { dg-error "no match" }
> +  x >> x; // { dg-error "no match" }
> +  x && x; // { dg-error "no match" }
> +  x || x; // { dg-error "no match" }
> +  x, x;
> +
> +  x == x; // { dg-error "no match" }
> +  x != x; // { dg-error "no match" }
> +  x < x; // { dg-error "no match" }
> +  x > x; // { dg-error "no match" }
> +  x <= x; // { dg-error "no match" }
> +  x >= x; // { dg-error "no match" }
> +#if __cplusplus > 201703L
> +  x <=> x; // { dg-error "no match" "" { target c++20 } }
> +#endif
> +
> +  x += x; // { dg-error "no match" }
> +  x -= x; // { dg-error "no match" }
> +  x *= x; // { dg-error "no match" }
> +  x /= x; // { dg-error "no match" }
> +  x %= x; // { dg-error "no match" }
> +  x |= x; // { dg-error "no match" }
> +  x ^= x; // { dg-error "no match" }
> +  x <<= x; // { dg-error "no match" }
> +  x >>= x; // { dg-error "no match" }
> +}
> +
> +namespace N { struct A { }; }
> +
> +void operator+(N::A);
> +void operator-(N::A);
> +void operator*(N::A);
> +void operator~(N::A);
> +#if __cplusplus >= 201103L
> +void operator&(N::A) = delete;
> +#else
> +void operator&(N::A);
> +#endif
> +void operator!(N::A);
> +void operator++(N::A);
> +void operator--(N::A);
> +void operator++(N::A, int);
> +void operator--(N::A, int);
> +
> +void operator->*(N::A, N::A);
> +void operator/(N::A, N::A);
> +void operator*(N::A, N::A);
> +void operator+(N::A, N::A);
> +void operator-(N::A, N::A);
> +void operator%(N::A, N::A);
> +void operator&(N::A, N::A);
> +void operator|(N::A, N::A);
> +void operator^(N::A, N::A);
> +void operator<<(N::A, N::A);
> +void operator>>(N::A, N::A);
> +void operator&&(N::A, N::A);
> +void operator||(N::A, N::A);
> +#if __cplusplus >= 201103L
> +void operator,(N::A, N::A) = delete;
> +#else
> +void operator,(N::A, N::A);
> +#endif
> +
> +void operator==(N::A, N::A);
> +void operator!=(N::A, N::A);
> +void operator<(N::A, N::A);
> +void operator>(N::A, N::A);
> +void operator<=(N::A, N::A);
> +void operator>=(N::A, N::A);
> +#if __cplusplus > 201703L
> +void operator<=>(N::A, N::A);
> +#endif
> +
> +void operator+=(N::A, N::A);
> +void operator-=(N::A, N::A);
> +void operator*=(N::A, N::A);
> +void operator/=(N::A, N::A);
> +void operator%=(N::A, N::A);
> +void operator|=(N::A, N::A);
> +void operator^=(N::A, N::A);
> +void operator<<=(N::A, N::A);
> +void operator>>=(N::A, N::A);
> +
> +int main() {
> +  f(N::A());
> +}
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-05-10 14:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10  2:33 [PATCH] c++: dependent operator expression lookup [PR51577] Patrick Palka
2021-05-10 14:51 ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).