diff mbox

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

Message ID 54f8da9d-0c1a-0a9d-a5ef-2ecb6cdc28c4@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Dec. 21, 2016, 8:27 p.m. UTC
This patch addresses bug 61636, which is an ICE during generic lambda 
instantiation due to unexpected this capture.

The problem is that a generic lambda's closure type is not a template, 
only the function operator is.  Thus this capture has to be determined 
at parsing time (and anyway, it'd be a weird instantiation if this 
capture was determined so late).

However, in a call with dependent arguments, overload resolution is 
deferred until instantiation time.  The patch simply adds a call to 
maybe_resolve_dummy when building the call expression, where lookup 
found a set of member functions.  (I was wrong about koenig lookup 
happening later, because:
	/* Do not do argument dependent lookup if regular
	   lookup finds a member function or a block-scope
	   function declaration.  [basic.lookup.argdep]/3  */

However, the std doesn't discuss this case, nor say how hard the 
compiler has to go to determine whether this capture might be needed. 
For instance, it could go as far as checking that there exists at least 
one non-static member function with an acceptable number of arguments. 
I don't do that here -- perhaps a DR is needed? From Adam's data it this 
is at least as good as Clang.

In template instantiation, we usually check in a dependent base to give 
good diagnostics and try and recover.  But that's another path to the 
same ICE and it seemed to me better to just give an error for this case, 
as there can't be legacy code using it.

ok?

nathan
diff mbox

Patch

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

	cp/
	PR c++/61636
	* 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.


2016-12-21  Nathan Sidwell  <nathan@acm.org>
	    Adam Butcher  <adam@jessamine.co.uk>
	testsuite/

	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/parser.c
===================================================================
--- cp/parser.c	(revision 243804)
+++ cp/parser.c	(working copy)
@@ -6971,6 +6971,14 @@  cp_parser_postfix_expression (cp_parser
 			|| type_dependent_expression_p (fn)
 			|| any_type_dependent_arguments_p (args)))
 		  {
+		    /* In a generic lambda we now speculatively
+		       capture this, because the eventual call might
+		       need it, and it'll be too late then.  The std
+		       doesn't specify exactly how hard we have to
+		       work to figure out whether we need to capture
+		       in this case.  */
+		    maybe_resolve_dummy (instance, true);
+
 		    postfix_expression
 		      = build_nt_call_vec (postfix_expression, args);
 		    release_tree_vector (args);
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 243804)
+++ cp/pt.c	(working copy)
@@ -16896,19 +16896,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))
@@ -16931,7 +16946,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,16 @@ 
+// PR c++/61636
+// { dg-do compile { target c++14 } }
+
+// ICE because we figure this capture too late.
+
+struct A {
+  void b ();
+  void Foo (int);
+};
+
+void A::b() {
+
+  auto lam = [&](auto asdf) { Foo (asdf); };
+
+  lam (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,62 @@ 
+// 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) {}
+};
+
+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;
+
+  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)
+
+  // 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 ();