[C++ PATCH] PR c++/88123 - lambda and using-directive.

[C++ PATCH] PR c++/88123 - lambda and using-directive.

[Jason Merrill]

For named function calls in a template, the result of unqualified lookup is
safed in CALL_EXPR_FN.  But for operator expressions, no unqualified lookup
is performed until we know whether the operands have class type.  So when we
see in a lambda a use of an operator that might be overloaded, we need to do
that lookup then and save it away somewhere.  One possibility would be in
the expression, but we can't really add extra conditional operands to
standard tree codes.  I mostly implemented another approach using a new
WITH_LOOKUP_EXPR code, but teaching everywhere how to handle a new tree code
is always complicated.  Then it occurred to me that we could associate the
lookups with the function, which is both simpler and smaller.  So this patch
stores any operator bindings needed by a lambda function in an internal
attribute on the lambda call operator.

Tested x86_64-pc-linux-gnu, applying to trunk.  Nathan, does slipping these
bindings into the sk_function_parms binding level this way make sense to you?

	* name-lookup.c (op_unqualified_lookup)
	(maybe_save_operator_binding, discard_operator_bindings)
	(push_operator_bindings): New.
	* typeck.c (build_x_binary_op, build_x_unary_op): Call
	maybe_save_operator_binding.
	* decl.c (start_preparsed_function): Call push_operator_bindings.
	* tree.c (cp_free_lang_data): Call discard_operator_bindings.
---
 gcc/cp/name-lookup.h                          |  3 +
 gcc/cp/decl.c                                 |  2 +
 gcc/cp/name-lookup.c                          | 99 +++++++++++++++++++
 gcc/cp/tree.c                                 |  2 +
 gcc/cp/typeck.c                               | 12 ++-
 .../g++.dg/cpp1y/lambda-generic-using1.C      | 29 ++++++
 gcc/cp/ChangeLog                              |  9 ++
 7 files changed, 154 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-generic-using1.C

diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index 36816df5ada..a47486d1b8a 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -330,5 +330,8 @@ extern void push_nested_namespace (tree);
 extern void pop_nested_namespace (tree);
 extern void push_to_top_level (void);
 extern void pop_from_top_level (void);
+extern void maybe_save_operator_binding (tree);
+extern void push_operator_bindings (void);
+extern void discard_operator_bindings (tree);
 
 #endif /* GCC_CP_NAME_LOOKUP_H */
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 173758feddf..0187db5ff1c 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -15553,6 +15553,8 @@ start_preparsed_function (tree decl1, tree attrs, int flags)
 
   store_parm_decls (current_function_parms);
 
+  push_operator_bindings ();
+
   if (!processing_template_decl
       && (flag_lifetime_dse > 1)
       && DECL_CONSTRUCTOR_P (decl1)
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 1ddcde26ef4..2ba888fd1c2 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -7556,4 +7556,103 @@ cp_emit_debug_info_for_using (tree t, tree context)
     }
 }
 
+/* Return the result of unqualified lookup for the overloaded operator
+   designated by CODE, if we are in a template and the binding we find is
+   not.  */
+
+static tree
+op_unqualified_lookup (tree fnname)
+{
+  if (cxx_binding *binding = IDENTIFIER_BINDING (fnname))
+    {
+      cp_binding_level *l = binding->scope;
+      while (l && !l->this_entity)
+	l = l->level_chain;
+      if (l && uses_template_parms (l->this_entity))
+	/* Don't preserve decls from an uninstantiated template,
+	   wait until that template is instantiated.  */
+	return NULL_TREE;
+    }
+  tree fns = lookup_name (fnname);
+  if (fns && fns == get_global_binding (fnname))
+    /* The instantiation can find these.  */
+    return NULL_TREE;
+  return fns;
+}
+
+/* E is an expression representing an operation with dependent type, so we
+   don't know yet whether it will use the built-in meaning of the operator or a
+   function.  Remember declarations of that operator in scope.  */
+
+const char *const op_bind_attrname = "operator bindings";
+
+void
+maybe_save_operator_binding (tree e)
+{
+  /* This is only useful in a generic lambda.  */
+  if (!processing_template_decl)
+    return;
+  tree cfn = current_function_decl;
+  if (!cfn)
+    return;
+
+  /* Let's only do this for generic lambdas for now, we could do it for all
+     function templates if we wanted to.  */
+  if (!current_lambda_expr())
+    return;
+
+  tree fnname = ovl_op_identifier (false, TREE_CODE (e));
+  if (!fnname)
+    return;
+
+  tree attributes = DECL_ATTRIBUTES (cfn);
+  tree attr = lookup_attribute (op_bind_attrname, attributes);
+  tree bindings = NULL_TREE;
+  tree fns = NULL_TREE;
+  if (attr)
+    {
+      bindings = TREE_VALUE (attr);
+      if (tree elt = purpose_member (fnname, bindings))
+	fns = TREE_VALUE (elt);
+    }
+
+  if (!fns && (fns = op_unqualified_lookup (fnname)))
+    {
+      bindings = tree_cons (fnname, fns, bindings);
+      if (attr)
+	TREE_VALUE (attr) = bindings;
+      else
+	DECL_ATTRIBUTES (cfn)
+	  = tree_cons (get_identifier (op_bind_attrname),
+		       bindings,
+		       attributes);
+    }
+}
+
+/* Called from cp_free_lang_data so we don't put this into LTO.  */
+
+void
+discard_operator_bindings (tree decl)
+{
+  DECL_ATTRIBUTES (decl) = remove_attribute (op_bind_attrname,
+					     DECL_ATTRIBUTES (decl));
+}
+
+/* Subroutine of start_preparsed_function: push the bindings we saved away in
+   maybe_save_op_lookup into the function parameter binding level.  */
+
+void
+push_operator_bindings ()
+{
+  tree decl1 = current_function_decl;
+  if (tree attr = lookup_attribute (op_bind_attrname,
+				    DECL_ATTRIBUTES (decl1)))
+    for (tree binds = TREE_VALUE (attr); binds; binds = TREE_CHAIN (binds))
+      {
+	tree name = TREE_PURPOSE (binds);
+	tree val = TREE_VALUE (binds);
+	push_local_binding (name, val, /*using*/true);
+      }
+}
+
 #include "gt-cp-name-lookup.h"
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index be33d4186f9..eca6b523c5f 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -5398,6 +5398,8 @@ cp_free_lang_data (tree t)
       DECL_EXTERNAL (t) = 1;
       TREE_STATIC (t) = 0;
     }
+  if (TREE_CODE (t) == FUNCTION_DECL)
+    discard_operator_bindings (t);
   if (TREE_CODE (t) == NAMESPACE_DECL)
     /* We do not need the leftover chaining of namespaces from the
        binding level.  */
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 9ceb7af0022..8d9224b668e 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4141,7 +4141,11 @@ build_x_binary_op (const op_location_t &loc, enum tree_code code, tree arg1,
     {
       if (type_dependent_expression_p (arg1)
 	  || type_dependent_expression_p (arg2))
-	return build_min_nt_loc (loc, code, arg1, arg2);
+	{
+	  expr = build_min_nt_loc (loc, code, arg1, arg2);
+	  maybe_save_operator_binding (expr);
+	  return expr;
+	}
       arg1 = build_non_dependent_expr (arg1);
       arg2 = build_non_dependent_expr (arg2);
     }
@@ -5725,7 +5729,11 @@ build_x_unary_op (location_t loc, enum tree_code code, cp_expr xarg,
   if (processing_template_decl)
     {
       if (type_dependent_expression_p (xarg))
-	return build_min_nt_loc (loc, code, xarg.get_value (), NULL_TREE);
+	{
+	  tree e = build_min_nt_loc (loc, code, xarg.get_value (), NULL_TREE);
+	  maybe_save_operator_binding (e);
+	  return e;
+	}
 
       xarg = build_non_dependent_expr (xarg);
     }
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-using1.C b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-using1.C
new file mode 100644
index 00000000000..2912a754a81
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-using1.C
@@ -0,0 +1,29 @@
+// PR c++/88123
+// { dg-do compile { target c++14 } }
+
+struct bar {};
+struct baz {};
+struct baq {};
+
+namespace foo
+{
+  void operator+(bar);
+} // namespace foo
+
+namespace foo2
+{
+  void operator-(baz);
+}  
+
+auto fn() {
+  using foo::operator+;
+  using namespace foo2;
+  extern void operator!(baq);
+  return [](auto x, auto y, auto z) { +x; -y; !z; };
+}
+
+int main()
+{
+  auto l = fn();
+  l(bar(),baz(),baq());
+}
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 94e278dc944..ae5735d30d2 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,5 +1,14 @@
 2019-03-07  Jason Merrill  <jason@redhat.com>
 
+	PR c++/88123 - lambda and using-directive.
+	* name-lookup.c (op_unqualified_lookup)
+	(maybe_save_operator_binding, discard_operator_bindings)
+	(push_operator_bindings): New.
+	* typeck.c (build_x_binary_op, build_x_unary_op): Call
+	maybe_save_operator_binding.
+	* decl.c (start_preparsed_function): Call push_operator_bindings.
+	* tree.c (cp_free_lang_data): Call discard_operator_bindings.
+
 	PR c++/88820 - ICE with CTAD and member template used in DMI.
 	* pt.c (do_class_deduction): Handle parm used as its own arg.
 

base-commit: 6425ec24a4907a71234231d37cd8421d1bf13585
-- 
2.20.1

Re: [C++ PATCH] PR c++/88123 - lambda and using-directive.

[Nathan Sidwell]

On 3/7/19 9:54 PM, Jason Merrill wrote:
> For named function calls in a template, the result of unqualified lookup is
> safed in CALL_EXPR_FN.  But for operator expressions, no unqualified lookup
> is performed until we know whether the operands have class type.  So when we
> see in a lambda a use of an operator that might be overloaded, we need to do
> that lookup then and save it away somewhere.  One possibility would be in
> the expression, but we can't really add extra conditional operands to
> standard tree codes.  I mostly implemented another approach using a new
> WITH_LOOKUP_EXPR code, but teaching everywhere how to handle a new tree code
> is always complicated.  Then it occurred to me that we could associate the
> lookups with the function, which is both simpler and smaller.  So this patch
> stores any operator bindings needed by a lambda function in an internal
> attribute on the lambda call operator.
> 
> Tested x86_64-pc-linux-gnu, applying to trunk.  Nathan, does slipping these
> bindings into the sk_function_parms binding level this way make sense to you?

oo sneaky, I like it.  Perhaps a comment explaining the surprising scope 
these are being pushed into?

nathan

-- 
Nathan Sidwell

Re: [C++ PATCH] PR c++/88123 - lambda and using-directive.

[Jason Merrill]

On Fri, Mar 8, 2019 at 8:04 AM Nathan Sidwell <nathan@acm.org> wrote:
>
> On 3/7/19 9:54 PM, Jason Merrill wrote:
> > For named function calls in a template, the result of unqualified lookup is
> > safed in CALL_EXPR_FN.  But for operator expressions, no unqualified lookup
> > is performed until we know whether the operands have class type.  So when we
> > see in a lambda a use of an operator that might be overloaded, we need to do
> > that lookup then and save it away somewhere.  One possibility would be in
> > the expression, but we can't really add extra conditional operands to
> > standard tree codes.  I mostly implemented another approach using a new
> > WITH_LOOKUP_EXPR code, but teaching everywhere how to handle a new tree code
> > is always complicated.  Then it occurred to me that we could associate the
> > lookups with the function, which is both simpler and smaller.  So this patch
> > stores any operator bindings needed by a lambda function in an internal
> > attribute on the lambda call operator.
> >
> > Tested x86_64-pc-linux-gnu, applying to trunk.  Nathan, does slipping these
> > bindings into the sk_function_parms binding level this way make sense to you?
>
> oo sneaky, I like it.  Perhaps a comment explaining the surprising scope
> these are being pushed into?

The comment for push_operator_bindings already says "push the bindings
we saved away in maybe_save_op_lookup into the function parameter
binding level", are you thinking of something else/more detailed?

Jason

Re: [C++ PATCH] PR c++/88123 - lambda and using-directive.

[Nathan Sidwell]

On 3/8/19 10:30 AM, Jason Merrill wrote:

> The comment for push_operator_bindings already says "push the bindings
> we saved away in maybe_save_op_lookup into the function parameter
> binding level", are you thinking of something else/more detailed?

good enough, thanks


-- 
Nathan Sidwell