diff mbox

[C++] c++/61636 generic lambdas and this capture

Message ID 65f1a240-adf2-4b04-5839-3f378de40f1d@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Jan. 13, 2017, 1:33 p.m. UTC
On 12/21/2016 03:27 PM, Nathan Sidwell wrote:
> This patch addresses bug 61636, which is an ICE during generic lambda
> instantiation due to unexpected this capture.

Here's an updated patch.
Firstly I added the worker function to lambda.c, with a more descriptive 
name.  Secondly, this implements a check that the overload set contains 
at least one non-static member fn.  That's what Clang does, and 
discussion on the std list seems to be heading that way as the right 
approach.

ok?

nathan

Comments

Jason Merrill Jan. 17, 2017, 4:57 p.m. UTC | #1
On 01/13/2017 08:33 AM, Nathan Sidwell wrote:
> 	* lambda.c (resolvable_dummy): New, broken out of ...

Maybe resolvable_dummy_lambda, since that's what it returns?

OK with that change.

Jason
diff mbox

Patch

2016-12-21  Nathan Sidwell  <nathan@acm.org>

	PR c++/61636
	* cp-tree.h (maybe_generic_this_capture): Declare.
	* lambda.c (resolvable_dummy): New, broken out of ...
	(maybe_resolve_dummy): ... here.  Call it.
	(maybe_generic_this_capture): New.
	* parser.c (cp_parser_postfix_expression): Speculatively capture
	this in generic lambda in unresolved member function call.
	* pt.c (tsubst_copy_and_build): Force hard error from failed
	member function lookup in generic lambda.

	PR c++/61636
	* g++.dg/cpp1y/pr61636-1.C: New.
	* g++.dg/cpp1y/pr61636-2.C: New.
	* g++.dg/cpp1y/pr61636-3.C: New.

Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 244382)
+++ cp/cp-tree.h	(working copy)
@@ -6551,6 +6551,7 @@  extern bool is_capture_proxy			(tree);
 extern bool is_normal_capture_proxy             (tree);
 extern void register_capture_members		(tree);
 extern tree lambda_expr_this_capture            (tree, bool);
+extern void maybe_generic_this_capture		(tree, tree);
 extern tree maybe_resolve_dummy			(tree, bool);
 extern tree current_nonlambda_function		(void);
 extern tree nonlambda_method_basetype		(void);
Index: cp/lambda.c
===================================================================
--- cp/lambda.c	(revision 244382)
+++ cp/lambda.c	(working copy)
@@ -793,16 +793,14 @@  lambda_expr_this_capture (tree lambda, b
   return result;
 }
 
-/* We don't want to capture 'this' until we know we need it, i.e. after
-   overload resolution has chosen a non-static member function.  At that
-   point we call this function to turn a dummy object into a use of the
-   'this' capture.  */
+/* Return the current LAMBDA_EXPR, if this is a resolvable dummy
+   object.  NULL otherwise..  */
 
-tree
-maybe_resolve_dummy (tree object, bool add_capture_p)
+static tree
+resolvable_dummy (tree object)
 {
   if (!is_dummy_object (object))
-    return object;
+    return NULL_TREE;
 
   tree type = TYPE_MAIN_VARIANT (TREE_TYPE (object));
   gcc_assert (!TYPE_PTR_P (type));
@@ -812,18 +810,55 @@  maybe_resolve_dummy (tree object, bool a
       && LAMBDA_TYPE_P (current_class_type)
       && lambda_function (current_class_type)
       && DERIVED_FROM_P (type, current_nonlambda_class_type ()))
-    {
-      /* In a lambda, need to go through 'this' capture.  */
-      tree lam = CLASSTYPE_LAMBDA_EXPR (current_class_type);
-      tree cap = lambda_expr_this_capture (lam, add_capture_p);
-      if (cap && cap != error_mark_node)
+    return CLASSTYPE_LAMBDA_EXPR (current_class_type);
+
+  return NULL_TREE;
+}
+
+/* We don't want to capture 'this' until we know we need it, i.e. after
+   overload resolution has chosen a non-static member function.  At that
+   point we call this function to turn a dummy object into a use of the
+   'this' capture.  */
+
+tree
+maybe_resolve_dummy (tree object, bool add_capture_p)
+{
+  if (tree lam = resolvable_dummy (object))
+    if (tree cap = lambda_expr_this_capture (lam, add_capture_p))
+      if (cap != error_mark_node)
 	object = build_x_indirect_ref (EXPR_LOCATION (object), cap,
 				       RO_NULL, tf_warning_or_error);
-    }
 
   return object;
 }
 
+/* When parsing a generic lambda containing an argument-dependent
+   member function call we defer overload resolution to instantiation
+   time.  But we have to know now whether to capture this or not.
+   Do that if FNS contains any non-static fns.
+   The std doesn't anticipate this case, but I expect this to be the
+   outcome of discussion.  */
+
+void
+maybe_generic_this_capture (tree object, tree fns)
+{
+  if (tree lam = resolvable_dummy (object))
+    if (!LAMBDA_EXPR_THIS_CAPTURE (lam))
+      {
+	/* We've not yet captured, so look at the function set of
+	   interest.  */
+	if (BASELINK_P (fns))
+	  fns = BASELINK_FUNCTIONS (fns);
+	for (; fns; fns = OVL_NEXT (fns))
+	  if (DECL_NONSTATIC_MEMBER_FUNCTION_P (OVL_CURRENT (fns)))
+	    {
+	      /* Found a non-static member.  Capture this.  */
+	      lambda_expr_this_capture (lam, true);
+	      break;
+	    }
+      }
+}
+
 /* Returns the innermost non-lambda function.  */
 
 tree
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 244382)
+++ cp/parser.c	(working copy)
@@ -6971,6 +6971,7 @@  cp_parser_postfix_expression (cp_parser
 			|| type_dependent_expression_p (fn)
 			|| any_type_dependent_arguments_p (args)))
 		  {
+		    maybe_generic_this_capture (instance, fn);
 		    postfix_expression
 		      = build_nt_call_vec (postfix_expression, args);
 		    release_tree_vector (args);
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 244382)
+++ cp/pt.c	(working copy)
@@ -17142,19 +17142,34 @@  tsubst_copy_and_build (tree t,
 
 		if (unq != function)
 		  {
-		    tree fn = unq;
-		    if (INDIRECT_REF_P (fn))
-		      fn = TREE_OPERAND (fn, 0);
-		    if (TREE_CODE (fn) == COMPONENT_REF)
-		      fn = TREE_OPERAND (fn, 1);
-		    if (is_overloaded_fn (fn))
-		      fn = get_first_fn (fn);
-		    if (permerror (EXPR_LOC_OR_LOC (t, input_location),
-				   "%qD was not declared in this scope, "
-				   "and no declarations were found by "
-				   "argument-dependent lookup at the point "
-				   "of instantiation", function))
+		    /* In a lambda fn, we have to be careful to not
+		       introduce new this captures.  Legacy code can't
+		       be using lambdas anyway, so it's ok to be
+		       stricter.  */
+		    bool in_lambda = (current_class_type
+				      && LAMBDA_TYPE_P (current_class_type));
+		    char const *msg = "%qD was not declared in this scope, "
+		      "and no declarations were found by "
+		      "argument-dependent lookup at the point "
+		      "of instantiation";
+
+		    bool diag = true;
+		    if (in_lambda)
+		      error_at (EXPR_LOC_OR_LOC (t, input_location),
+				msg, function);
+		    else
+		      diag = permerror (EXPR_LOC_OR_LOC (t, input_location),
+					msg, function);
+		    if (diag)
 		      {
+			tree fn = unq;
+			if (INDIRECT_REF_P (fn))
+			  fn = TREE_OPERAND (fn, 0);
+			if (TREE_CODE (fn) == COMPONENT_REF)
+			  fn = TREE_OPERAND (fn, 1);
+			if (is_overloaded_fn (fn))
+			  fn = get_first_fn (fn);
+
 			if (!DECL_P (fn))
 			  /* Can't say anything more.  */;
 			else if (DECL_CLASS_SCOPE_P (fn))
@@ -17177,7 +17192,13 @@  tsubst_copy_and_build (tree t,
 			  inform (DECL_SOURCE_LOCATION (fn),
 				  "%qD declared here, later in the "
 				  "translation unit", fn);
+			if (in_lambda)
+			  {
+			    release_tree_vector (call_args);
+			    RETURN (error_mark_node);
+			  }
 		      }
+
 		    function = unq;
 		  }
 	      }
Index: testsuite/g++.dg/cpp1y/pr61636-1.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr61636-1.C	(revision 0)
+++ testsuite/g++.dg/cpp1y/pr61636-1.C	(working copy)
@@ -0,0 +1,31 @@ 
+// PR c++/61636
+// { dg-do compile { target c++14 } }
+
+// ICE because we figure this capture too late.
+
+struct Base
+{
+  void Bar (int);
+};
+
+struct A : Base {
+  void b ();
+  void Foo (int);
+  using Base::Bar;
+  template <typename T> void Baz (T);
+};
+
+void A::b() {
+
+  auto lam = [&](auto asdf) { Foo (asdf); };
+
+  lam (0);
+
+  auto lam1 = [&](auto asdf) { Bar (asdf); };
+
+  lam1 (0);
+
+  auto lam2 = [&](auto asdf) { Baz (asdf); };
+
+  lam2 (0);
+}
Index: testsuite/g++.dg/cpp1y/pr61636-2.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr61636-2.C	(revision 0)
+++ testsuite/g++.dg/cpp1y/pr61636-2.C	(working copy)
@@ -0,0 +1,72 @@ 
+// PR c++/61636
+// { dg-do run { target c++14 } }
+
+// Check we don't capture this (too) unnecessarily
+
+struct A {
+  int b ();
+  void f (int) {}
+  static void f (double) {}
+
+  static void g (int) {}
+  static void g (double) {}
+};
+
+struct O {
+  void x (int) {}
+  static void x (double) {}
+};
+
+namespace N {
+  void y (double) {}
+}
+
+int Check (bool expect, unsigned size)
+{
+  return (expect ? sizeof (void *) : 1) != size;
+}
+
+int A::b() {
+  int r = 0;
+
+  // one of the functions is non-static
+  auto l0 = [&](auto z) { f (z); };
+  r += Check (true, sizeof l0);
+  l0(0.0); // doesn't need this capture for A::f(double), but too late
+  l0 (0); // Needs this capture for A::f(int)
+
+  // no fn is non-static.
+  auto l00 = [&](auto z) { g (z); };
+  r += Check (false, sizeof l00);
+  l00(0.0); 
+  l00 (0);
+
+  // sizeof isn't an evaluation context, so no this capture
+  auto l1 = [&](auto z) { sizeof (f (z), 1); };
+  r += Check (false, sizeof l1);
+  l1(0.0); l1 (0); 
+
+  auto l2 = [&](auto) { f (2.4); };
+  auto l3 = [&](auto) { f (0); };
+  l2(0); l3(0); l2(0.0); l3 (0.0);
+  r += Check (false, sizeof l2);
+  r += Check (true, sizeof l3);
+
+  auto l4 = [&](auto) { O::x (2.4); };
+  auto l5 = [&](auto) { N::y (2.4); };
+  auto l6 = [&](auto) { };
+  l4(0); l5(0); l6(0);
+  l4(0.0); l5(0.0); l6(0.0);
+  r += Check (false, sizeof l4);
+  r += Check (false, sizeof l5);
+  r += Check (false, sizeof l6);
+
+  return r;
+}
+
+int main ()
+{
+  A a;
+
+  return a.b () ? 1 : 0;
+}
Index: testsuite/g++.dg/cpp1y/pr61636-3.C
===================================================================
--- testsuite/g++.dg/cpp1y/pr61636-3.C	(revision 0)
+++ testsuite/g++.dg/cpp1y/pr61636-3.C	(working copy)
@@ -0,0 +1,25 @@ 
+// PR c++/61636
+// { dg-do compile { target c++14 } }
+// permissiveness doesn't make this permitted
+// { dg-additional-options "-fpermissive" }
+
+// ICE because we attempt to use dependent Foo during error recovery
+// and die with an unexpected this capture need.
+
+template <typename T> struct Base
+{
+  void Foo (int);
+};
+
+template <typename T> struct A : Base<T> {
+  void b ();
+};
+
+template <typename T> void A<T>::b() {
+
+  auto lam = [&](auto asdf) { Foo (asdf); }; // { dg-error "not declared" }
+
+  lam (T(0));
+}
+
+template void A<int>::b ();