C++ PATCH for c++/84338, wrong variadic sizeof

Message ID CADzB+2nnsdARLZo2J26i7998QsGLKpPw5b-+pRC20TJ_BM6ptA@mail.gmail.com
State New
Headers show
Series
  • C++ PATCH for c++/84338, wrong variadic sizeof
Related show

Commit Message

Jason Merrill Feb. 13, 2018, 2:07 p.m.
Here sizeof... was improperly being resolved early on an argument pack
consisting of a single pack expansion, when the sizeof... is still
dependent.  This turned out to be because of our handling of partial
instantiation of a function parameter pack: I had previously changed
extract_fnparm_pack to wrap such a partially instantiated pack in an
EXPR_PACK_EXPANSION, but later reverted that because it was breaking
things.  That breakage turns out to have been because the uses of
ARGUMENT_PACK_EXTRACT_ARG for PARM_DECL and VAR_DECL were missing the
code to strip a pack expansion that we had for template parameters;
moving all of that into a function rather than a macro fixed that, and
so now we can wrap the parameter pack again, which fixes the testcase.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 0978de34113258d973e3a78dad466c3d602a9130
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Feb 12 17:04:56 2018 -0500

            PR c++/84338 - wrong variadic sizeof.
    
            * pt.c (argument_pack_select_arg): Like the macro, but look through
            a pack expansion.
            (tsubst, tsubst_copy, dependent_template_arg_p): Use it.
            (extract_fnparm_pack): Do make_pack_expansion.
            (extract_locals_r): Do strip a pack expansion.
            * cp-tree.h (ARGUMENT_PACK_SELECT_ARG): Remove.

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index a6c75aed33d..9a9e9f0bbcb 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3558,12 +3558,6 @@  extern void decl_shadowed_for_var_insert (tree, tree);
 #define ARGUMENT_PACK_SELECT_INDEX(NODE)				\
   (((struct tree_argument_pack_select *)ARGUMENT_PACK_SELECT_CHECK (NODE))->index)
   
-/* In an ARGUMENT_PACK_SELECT, the actual underlying argument that the
-   ARGUMENT_PACK_SELECT represents. */
-#define ARGUMENT_PACK_SELECT_ARG(NODE)					\
-  TREE_VEC_ELT (ARGUMENT_PACK_ARGS (ARGUMENT_PACK_SELECT_FROM_PACK (NODE)), \
-	        ARGUMENT_PACK_SELECT_INDEX (NODE))
-
 #define FOLD_EXPR_CHECK(NODE)						\
   TREE_CHECK4 (NODE, UNARY_LEFT_FOLD_EXPR, UNARY_RIGHT_FOLD_EXPR,	\
 	       BINARY_LEFT_FOLD_EXPR, BINARY_RIGHT_FOLD_EXPR)
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index b58c60f0dcb..a83b7073d20 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3394,6 +3394,34 @@  get_template_argument_pack_elems (const_tree t)
   return ARGUMENT_PACK_ARGS (t);
 }
 
+/* In an ARGUMENT_PACK_SELECT, the actual underlying argument that the
+   ARGUMENT_PACK_SELECT represents. */
+
+static tree
+argument_pack_select_arg (tree t)
+{
+  tree args = ARGUMENT_PACK_ARGS (ARGUMENT_PACK_SELECT_FROM_PACK (t));
+  tree arg = TREE_VEC_ELT (args, ARGUMENT_PACK_SELECT_INDEX (t));
+
+  /* If the selected argument is an expansion E, that most likely means we were
+     called from gen_elem_of_pack_expansion_instantiation during the
+     substituting of an argument pack (of which the Ith element is a pack
+     expansion, where I is ARGUMENT_PACK_SELECT_INDEX) into a pack expansion.
+     In this case, the Ith element resulting from this substituting is going to
+     be a pack expansion, which pattern is the pattern of E.  Let's return the
+     pattern of E, and gen_elem_of_pack_expansion_instantiation will build the
+     resulting pack expansion from it.  */
+  if (PACK_EXPANSION_P (arg))
+    {
+      /* Make sure we aren't throwing away arg info.  */
+      gcc_assert (!PACK_EXPANSION_EXTRA_ARGS (arg));
+      arg = PACK_EXPANSION_PATTERN (arg);
+    }
+
+  return arg;
+}
+
+
 /* True iff FN is a function representing a built-in variadic parameter
    pack.  */
 
@@ -10933,7 +10961,12 @@  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_VEC_ELT (parmvec, i) = spec_parm;
+    {
+      tree elt = spec_parm;
+      if (DECL_PACK_P (elt))
+	elt = make_pack_expansion (elt);
+      TREE_VEC_ELT (parmvec, i) = elt;
+    }
 
   /* Build the argument packs.  */
   SET_ARGUMENT_PACK_ARGS (argpack, parmvec);
@@ -11388,7 +11421,8 @@  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);
-	  spec = TREE_VEC_ELT (args, 0);
+	  tree arg = TREE_VEC_ELT (args, 0);
+	  spec = PACK_EXPANSION_PATTERN (arg);
 	}
       *extra = tree_cons (*tp, spec, *extra);
     }
@@ -13747,29 +13781,9 @@  tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	  {
 	    arg = TMPL_ARG (args, level, idx);
 
+	    /* See through ARGUMENT_PACK_SELECT arguments. */
 	    if (arg && TREE_CODE (arg) == ARGUMENT_PACK_SELECT)
-	      {
-		/* See through ARGUMENT_PACK_SELECT arguments. */
-		arg = ARGUMENT_PACK_SELECT_ARG (arg);
-		/* If the selected argument is an expansion E, that most
-		   likely means we were called from
-		   gen_elem_of_pack_expansion_instantiation during the
-		   substituting of pack an argument pack (which Ith
-		   element is a pack expansion, where I is
-		   ARGUMENT_PACK_SELECT_INDEX) into a pack expansion.
-		   In this case, the Ith element resulting from this
-		   substituting is going to be a pack expansion, which
-		   pattern is the pattern of E.  Let's return the
-		   pattern of E, and
-		   gen_elem_of_pack_expansion_instantiation will
-		   build the resulting pack expansion from it.  */
-		if (PACK_EXPANSION_P (arg))
-		  {
-		    /* Make sure we aren't throwing away arg info.  */
-		    gcc_assert (!PACK_EXPANSION_EXTRA_ARGS (arg));
-		    arg = PACK_EXPANSION_PATTERN (arg);
-		  }
-	      }
+	      arg = argument_pack_select_arg (arg);
 	  }
 
 	if (arg == error_mark_node)
@@ -14734,7 +14748,7 @@  tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	}
       
       if (TREE_CODE (r) == ARGUMENT_PACK_SELECT)
-	r = ARGUMENT_PACK_SELECT_ARG (r);
+	r = argument_pack_select_arg (r);
       if (!mark_used (r, complain) && !(complain & tf_error))
 	return error_mark_node;
       return r;
@@ -14866,7 +14880,7 @@  tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 		register_local_specialization (r, t);
 	    }
 	  if (TREE_CODE (r) == ARGUMENT_PACK_SELECT)
-	    r = ARGUMENT_PACK_SELECT_ARG (r);
+	    r = argument_pack_select_arg (r);
 	}
       else
 	r = t;
@@ -24701,7 +24715,7 @@  dependent_template_arg_p (tree arg)
     return true;
 
   if (TREE_CODE (arg) == ARGUMENT_PACK_SELECT)
-    arg = ARGUMENT_PACK_SELECT_ARG (arg);
+    arg = argument_pack_select_arg (arg);
 
   if (TREE_CODE (arg) == TEMPLATE_TEMPLATE_PARM)
     return true;
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic13.C b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic13.C
new file mode 100644
index 00000000000..92fd34cb268
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic13.C
@@ -0,0 +1,15 @@ 
+// PR c++/84338
+// { dg-do compile { target c++14 } }
+
+template < typename ... T >
+auto f(T ... i){
+    [](auto ... i){
+        // // wrongly true in current trunk
+        // static_assert(sizeof...(i) == 1, "");
+        static_assert(sizeof...(i) == 2, "");
+    }(i ...);
+}
+
+int main(){
+    f(0, 1);
+}