C++ PATCH for c++/84036, ICE with variadic lambda capture

Message ID CADzB+2n0g5++2V+L=J54psHf4e3t7n2nEcmb=-7A2oqf2nRiYw@mail.gmail.com
State New
Headers show
Series
  • C++ PATCH for c++/84036, ICE with variadic lambda capture
Related show

Commit Message

Jason Merrill Feb. 12, 2018, 1:21 a.m.
The old lambda model handled variadic capture by focusing on the
FIELD_DECL rather than trying to map between capture proxies.  The new
model relies more on capture proxies, so it makes sense to use them
more for variadic capture as well.  So with this patch we treat a
variadic capture proxy as a pack, rather than the field.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit d6e4c6a3af04599f408b04e630b853d5a907beac
Author: Jason Merrill <jason@redhat.com>
Date:   Thu Feb 8 16:18:33 2018 -0500

            PR c++/84036 - ICE with variadic capture.
    
            Handle variadic capture proxies more like non-variadic.
            * lambda.c (build_capture_proxy): Remove workaround.
            * pt.c (find_parameter_packs_r): The proxy is a pack.
            (instantiate_class_template_1): Remove dead lambda code.
            (extract_fnparm_pack): Don't make_pack_expansion.
            (extract_locals_r): Don't strip a pack expansion.
            (tsubst_pack_expansion): Handle proxy packs.  Use
            PACK_EXPANSION_EXTRA_ARGS less.
            (tsubst_decl) [FIELD_DECL]: Don't register_specialization.
            (tsubst_copy) [FIELD_DECL]: Don't retrieve*_specialization.
            [VAR_DECL]: Handle ARGUMENT_PACK_SELECT.
            (tsubst_expr) [DECL_EXPR]: Handle proxy packs.
            (tsubst_copy_and_build) [VAR_DECL]: Handle proxy packs normally.

Comments

Jason Merrill March 7, 2018, 8:59 p.m. | #1
On Sun, Feb 11, 2018 at 8:21 PM, Jason Merrill <jason@redhat.com> wrote:
> The old lambda model handled variadic capture by focusing on the
> FIELD_DECL rather than trying to map between capture proxies.  The new
> model relies more on capture proxies, so it makes sense to use them
> more for variadic capture as well.  So with this patch we treat a
> variadic capture proxy as a pack, rather than the field.

Which makes the recently added is_capture_proxy_with_ref redundant.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 4f59367cbb4fe82c68f9df30e49af8a2a19dd6cf
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Mar 6 17:31:44 2018 -0500

            * lambda.c (is_capture_proxy_with_ref): Remove.
    
            * constexpr.c, expr.c, cp-tree.h, semantics.c: Adjust.

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index bd53bfbfe47..2c5a71f3ee5 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5429,7 +5429,7 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
     case VAR_DECL:
       if (DECL_HAS_VALUE_EXPR_P (t))
 	{
-	  if (now && is_capture_proxy_with_ref (t))
+	  if (now && is_normal_capture_proxy (t))
 	    {
 	      /* -- in a lambda-expression, a reference to this or to a
 		 variable with automatic storage duration defined outside that
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 8f3ec86e8ce..3a6f8f33e8d 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6896,7 +6896,6 @@ extern void insert_capture_proxy		(tree);
 extern void insert_pending_capture_proxies	(void);
 extern bool is_capture_proxy			(tree);
 extern bool is_normal_capture_proxy             (tree);
-extern bool is_capture_proxy_with_ref           (tree);
 extern void register_capture_members		(tree);
 extern tree lambda_expr_this_capture            (tree, bool);
 extern void maybe_generic_this_capture		(tree, tree);
diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
index b2c8cfaf88c..2e679868970 100644
--- a/gcc/cp/expr.c
+++ b/gcc/cp/expr.c
@@ -111,7 +111,7 @@ mark_use (tree expr, bool rvalue_p, bool read_p,
     {
     case VAR_DECL:
     case PARM_DECL:
-      if (rvalue_p && is_capture_proxy_with_ref (expr))
+      if (rvalue_p && is_normal_capture_proxy (expr))
 	{
 	  /* Look through capture by copy.  */
 	  tree cap = DECL_CAPTURED_VARIABLE (expr);
@@ -154,7 +154,7 @@ mark_use (tree expr, bool rvalue_p, bool read_p,
 	{
 	  /* Try to look through the reference.  */
 	  tree ref = TREE_OPERAND (expr, 0);
-	  if (rvalue_p && is_capture_proxy_with_ref (ref))
+	  if (rvalue_p && is_normal_capture_proxy (ref))
 	    {
 	      /* Look through capture by reference.  */
 	      tree cap = DECL_CAPTURED_VARIABLE (ref);
diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index 345b210e89c..094979e81a3 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -291,24 +291,13 @@ is_normal_capture_proxy (tree decl)
   return DECL_NORMAL_CAPTURE_P (val);
 }
 
-/* Returns true iff DECL is a capture proxy for which we can use
-   DECL_CAPTURED_VARIABLE.  In effect, this is a normal proxy other than a
-   nested capture of a function parameter pack.  */
-
-bool
-is_capture_proxy_with_ref (tree var)
-{
-  return (is_normal_capture_proxy (var) && DECL_LANG_SPECIFIC (var)
-	  && DECL_CAPTURED_VARIABLE (var));
-}
-
 /* VAR is a capture proxy created by build_capture_proxy; add it to the
    current function, which is the operator() for the appropriate lambda.  */
 
 void
 insert_capture_proxy (tree var)
 {
-  if (is_capture_proxy_with_ref (var))
+  if (is_normal_capture_proxy (var))
     {
       tree cap = DECL_CAPTURED_VARIABLE (var);
       if (CHECKING_P)
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 8a0096ddf92..bb8b5953539 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3332,7 +3332,7 @@ process_outer_var_ref (tree decl, tsubst_flags_t complain, bool odr_use)
     {
       /* Check whether we've already built a proxy.  */
       tree var = decl;
-      while (is_capture_proxy_with_ref (var))
+      while (is_normal_capture_proxy (var))
 	var = DECL_CAPTURED_VARIABLE (var);
       tree d = retrieve_local_specialization (var);

Patch

diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
index 2545eae9ce9..6b5bd800741 100644
--- a/gcc/cp/lambda.c
+++ b/gcc/cp/lambda.c
@@ -455,19 +455,11 @@  build_capture_proxy (tree member, tree init)
 	  STRIP_NOPS (init);
 	}
 
-      if (TREE_CODE (init) == COMPONENT_REF)
-	/* We're capturing a capture of a function parameter pack, and have
-	   lost track of the original variable.  It's not important to have
-	   DECL_CAPTURED_VARIABLE in this case, since a function parameter pack
-	   isn't a constant variable, so don't bother trying to set it.  */;
-      else
-	{
-	  gcc_assert (VAR_P (init) || TREE_CODE (init) == PARM_DECL);
-	  while (is_normal_capture_proxy (init))
-	    init = DECL_CAPTURED_VARIABLE (init);
-	  retrofit_lang_decl (var);
-	  DECL_CAPTURED_VARIABLE (var) = init;
-	}
+      gcc_assert (VAR_P (init) || TREE_CODE (init) == PARM_DECL);
+      while (is_normal_capture_proxy (init))
+	init = DECL_CAPTURED_VARIABLE (init);
+      retrofit_lang_decl (var);
+      DECL_CAPTURED_VARIABLE (var) = init;
     }
 
   if (name == this_identifier)
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 281604594ad..b58c60f0dcb 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3561,14 +3561,13 @@  find_parameter_packs_r (tree *tp, int *walk_subtrees, void* data)
 
       /* Look through a lambda capture proxy to the field pack.  */
     case VAR_DECL:
-      if (DECL_HAS_VALUE_EXPR_P (t))
-	{
-	  tree v = DECL_VALUE_EXPR (t);
-	  cp_walk_tree (&v,
-			&find_parameter_packs_r,
-			ppd, ppd->visited);
-	  *walk_subtrees = 0;
-	}
+      if (DECL_PACK_P (t))
+        {
+          /* We don't want to walk into the type of a variadic capture proxy,
+             because we don't want to see the type parameter pack.  */
+          *walk_subtrees = 0;
+	  parameter_pack_p = true;
+        }
       else if (variable_template_specialization_p (t))
 	{
 	  cp_walk_tree (&DECL_TI_ARGS (t),
@@ -10838,42 +10837,6 @@  instantiate_class_template_1 (tree type)
       c_inhibit_evaluation_warnings = saved_inhibit_evaluation_warnings;
     }
 
-  if (tree expr = CLASSTYPE_LAMBDA_EXPR (type))
-    {
-      tree decl = lambda_function (type);
-      if (decl)
-	{
-	  if (cxx_dialect >= cxx17)
-	    CLASSTYPE_LITERAL_P (type) = true;
-
-	  if (!DECL_TEMPLATE_INFO (decl)
-	      || DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl)) != decl)
-	    {
-	      /* Set function_depth to avoid garbage collection.  */
-	      ++function_depth;
-	      instantiate_decl (decl, /*defer_ok=*/false, false);
-	      --function_depth;
-	    }
-
-	  /* We need to instantiate the capture list from the template
-	     after we've instantiated the closure members, but before we
-	     consider adding the conversion op.  Also keep any captures
-	     that may have been added during instantiation of the op().  */
-	  tree tmpl_expr = CLASSTYPE_LAMBDA_EXPR (pattern);
-	  tree tmpl_cap
-	    = tsubst_copy_and_build (LAMBDA_EXPR_CAPTURE_LIST (tmpl_expr),
-				     args, tf_warning_or_error, NULL_TREE,
-				     false, false);
-
-	  LAMBDA_EXPR_CAPTURE_LIST (expr)
-	    = chainon (tmpl_cap, nreverse (LAMBDA_EXPR_CAPTURE_LIST (expr)));
-
-	  maybe_add_lambda_conv_op (type);
-	}
-      else
-	gcc_assert (errorcount);
-    }
-
   /* Set the file and line number information to whatever is given for
      the class itself.  This puts error messages involving generated
      implicit functions at a predictable point, and the same point
@@ -10970,12 +10933,7 @@  extract_fnparm_pack (tree tmpl_parm, tree *spec_p)
   parmvec = make_tree_vec (len);
   spec_parm = *spec_p;
   for (i = 0; i < len; i++, spec_parm = DECL_CHAIN (spec_parm))
-    {
-      tree elt = spec_parm;
-      if (DECL_PACK_P (elt))
-	elt = make_pack_expansion (elt);
-      TREE_VEC_ELT (parmvec, i) = elt;
-    }
+    TREE_VEC_ELT (parmvec, i) = spec_parm;
 
   /* Build the argument packs.  */
   SET_ARGUMENT_PACK_ARGS (argpack, parmvec);
@@ -11125,6 +11083,7 @@  gen_elem_of_pack_expansion_instantiation (tree pattern,
 
       /* Select the Ith argument from the pack.  */
       if (TREE_CODE (parm) == PARM_DECL
+	  || VAR_P (parm)
 	  || TREE_CODE (parm) == FIELD_DECL)
 	{
 	  if (index == 0)
@@ -11429,8 +11388,7 @@  extract_locals_r (tree *tp, int */*walk_subtrees*/, void *data)
 	  /* Pull out the actual PARM_DECL for the partial instantiation.  */
 	  tree args = ARGUMENT_PACK_ARGS (spec);
 	  gcc_assert (TREE_VEC_LENGTH (args) == 1);
-	  tree arg = TREE_VEC_ELT (args, 0);
-	  spec = PACK_EXPANSION_PATTERN (arg);
+	  spec = TREE_VEC_ELT (args, 0);
 	}
       *extra = tree_cons (*tp, spec, *extra);
     }
@@ -11551,8 +11509,12 @@  tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 	       where it isn't expected).  */
 	    unsubstituted_fn_pack = true;
 	}
-      else if (TREE_CODE (parm_pack) == FIELD_DECL)
-	arg_pack = tsubst_copy (parm_pack, args, complain, in_decl);
+      else if (is_normal_capture_proxy (parm_pack))
+	{
+	  arg_pack = retrieve_local_specialization (parm_pack);
+	  if (argument_pack_element_is_expansion_p (arg_pack, 0))
+	    unsubstituted_fn_pack = true;
+	}
       else
         {
 	  int idx;
@@ -11647,15 +11609,14 @@  tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 
   /* We cannot expand this expansion expression, because we don't have
      all of the argument packs we need.  */
-  if (use_pack_expansion_extra_args_p (packs, len, (unsubstituted_packs
-						    || unsubstituted_fn_pack)))
+  if (use_pack_expansion_extra_args_p (packs, len, unsubstituted_packs))
     {
       /* We got some full packs, but we can't substitute them in until we
 	 have values for all the packs.  So remember these until then.  */
 
       t = make_pack_expansion (pattern, complain);
       tree extra = args;
-      if (unsubstituted_fn_pack)
+      if (local_specializations)
 	if (tree locals = extract_local_specs (pattern))
 	  extra = tree_cons (NULL_TREE, extra, locals);
       PACK_EXPANSION_EXTRA_ARGS (t) = extra;
@@ -11713,6 +11674,7 @@  tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
       tree parm = TREE_PURPOSE (pack);
 
       if (TREE_CODE (parm) == PARM_DECL
+	  || VAR_P (parm)
 	  || TREE_CODE (parm) == FIELD_DECL)
         register_local_specialization (TREE_TYPE (pack), parm);
       else
@@ -12866,9 +12828,7 @@  tsubst_decl (tree t, tree args, tsubst_flags_t complain)
 	if (PACK_EXPANSION_P (TREE_TYPE (t)))
 	  {
 	    /* This field is a lambda capture pack.  Return a TREE_VEC of
-	       the expanded fields to instantiate_class_template_1 and
-	       store them in the specializations hash table as a
-	       NONTYPE_ARGUMENT_PACK so that tsubst_copy can find them.  */
+	       the expanded fields to instantiate_class_template_1.  */
             expanded_types = tsubst_pack_expansion (TREE_TYPE (t), args,
 						    complain, in_decl);
             if (TREE_CODE (expanded_types) == TREE_VEC)
@@ -12930,12 +12890,7 @@  tsubst_decl (tree t, tree args, tsubst_flags_t complain)
 	  }
 
 	if (vec)
-	  {
-	    r = vec;
-	    tree pack = make_node (NONTYPE_ARGUMENT_PACK);
-	    SET_ARGUMENT_PACK_ARGS (pack, vec);
-	    register_specialization (pack, t, args, false, 0);
-	  }
+	  r = vec;
       }
       break;
 
@@ -14827,31 +14782,6 @@  tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
       return t;
 
     case FIELD_DECL:
-      if (PACK_EXPANSION_P (TREE_TYPE (t)))
-	{
-	  /* Check for a local specialization set up by
-	     tsubst_pack_expansion.  */
-	  if (tree r = retrieve_local_specialization (t))
-	    {
-	      if (TREE_CODE (r) == ARGUMENT_PACK_SELECT)
-		r = ARGUMENT_PACK_SELECT_ARG (r);
-	      return r;
-	    }
-
-	  /* When retrieving a capture pack from a generic lambda, remove the
-	     lambda call op's own template argument list from ARGS.  Only the
-	     template arguments active for the closure type should be used to
-	     retrieve the pack specialization.  */
-	  if (LAMBDA_FUNCTION_P (current_function_decl)
-	      && (template_class_depth (DECL_CONTEXT (t))
-		  != TMPL_ARGS_DEPTH (args)))
-	    args = strip_innermost_template_args (args, 1);
-
-	  /* Otherwise return the full NONTYPE_ARGUMENT_PACK that
-	     tsubst_decl put in the hash table.  */
-	  return retrieve_specialization (t, args, 0);
-	}
-
       if (DECL_CONTEXT (t))
 	{
 	  tree ctx;
@@ -14935,6 +14865,8 @@  tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	      if (local_specializations)
 		register_local_specialization (r, t);
 	    }
+	  if (TREE_CODE (r) == ARGUMENT_PACK_SELECT)
+	    r = ARGUMENT_PACK_SELECT_ARG (r);
 	}
       else
 	r = t;
@@ -16104,20 +16036,24 @@  tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 	    else
 	      finish_local_using_decl (decl, scope, name);
 	  }
-	else if (DECL_PACK_P (decl))
-	  {
-	    /* Don't build up decls for a variadic capture proxy, we'll
-	       instantiate the elements directly as needed.  */
-	    break;
-	  }
 	else if (is_capture_proxy (decl)
 		 && !DECL_TEMPLATE_INSTANTIATION (current_function_decl))
 	  {
 	    /* We're in tsubst_lambda_expr, we've already inserted a new
 	       capture proxy, so look it up and register it.  */
-	    tree inst = lookup_name_real (DECL_NAME (decl), 0, 0,
-					  /*block_p=*/true, 0, LOOKUP_HIDDEN);
-	    gcc_assert (inst != decl && is_capture_proxy (inst));
+	    tree inst;
+	    if (DECL_PACK_P (decl))
+	      {
+		inst = (retrieve_local_specialization
+			(DECL_CAPTURED_VARIABLE (decl)));
+		gcc_assert (TREE_CODE (inst) == NONTYPE_ARGUMENT_PACK);
+	      }
+	    else
+	      {
+		inst = lookup_name_real (DECL_NAME (decl), 0, 0,
+					 /*block_p=*/true, 0, LOOKUP_HIDDEN);
+		gcc_assert (inst != decl && is_capture_proxy (inst));
+	      }
 	    register_local_specialization (inst, decl);
 	    break;
 	  }
@@ -18265,14 +18201,6 @@  tsubst_copy_and_build (tree t,
     case VAR_DECL:
       if (!args)
 	RETURN (t);
-      else if (DECL_PACK_P (t))
-	{
-	  /* We don't build decls for an instantiation of a
-	     variadic capture proxy, we instantiate the elements
-	     when needed.  */
-	  gcc_assert (DECL_HAS_VALUE_EXPR_P (t));
-	  return RECUR (DECL_VALUE_EXPR (t));
-	}
       /* Fall through */
 
     case PARM_DECL:
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic11.C b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic11.C
new file mode 100644
index 00000000000..01ef7c6e220
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic11.C
@@ -0,0 +1,21 @@ 
+// PR c++/84036
+// { dg-do compile { target c++14 } }
+
+template < typename... T > void sink(T ...){}
+
+template < typename T >
+auto f(T){
+  auto l = [](auto ... i){
+    [i ...]{
+      sink(i...);
+      [=]{ sink(i ...); }();
+    }();
+  };
+  l();
+  l(42);
+  l(0.1,0.2);
+}
+
+int main(){
+    f(0);
+}