Patchwork PR c++/56782 - Regression with empty pack expansions

login
register
mail settings
Submitter Dodji Seketeli
Date May 15, 2013, 2:33 p.m.
Message ID <877gj0pb2x.fsf@redhat.com>
Download mbox | patch
Permalink /patch/244091/
State New
Headers show

Comments

Dodji Seketeli - May 15, 2013, 2:33 p.m.
Hello,

In the example of the patch below, during the instantiation of
is_convertible at #1, we see at some point Tuple<>.  (Let's note '{}'
an empty argument pack.)  In that context, during the partial
specialization the member template

template<class... U>
Tuple<>::Tuple<U,
	       typename enable_if<and_<is_convertible<U, {}>...
                                      >::value,
                                  int
			         >::type
              >

Let's look at what happens to the expansion "is_convertible<U, {}>...."

To express the result of that expansion tsubst_pack_expansion receives
the expansion is_convertible<U, T>, with the argument list [{}].  This
function should detect that we have an empty argument pack for the
parameter pack T and no argument pack for the parameter pack U.  It
should thus return a pack expansion "is_convertible<U,T>..." that has this
information: "I have gotten an argument list, that is not complete
because U doesn't have any argument pack; the argument pack for T is
'{}', so I'll wait for the next time I am passed to
tsubst_pack_expansion with enough additional argument packs, to really
perform the substitution".  That information is conveyed by attaching
the the '{}' to the PACK_EXPANSION_EXTRA property of the pack expansion
returned by tsubst_pack_expansion.

The problem in this report is that we are not setting
PACK_EXPANSION_EXTRA when the non-complete argument pack list is made
of an empty argument pack, because use_pack_expansion_extra_args_p
doesn't detect this case.

Fixed thus.

gcc/cp/

	* pt.c (use_pack_expansion_extra_args_p): When at least a
	parameter pack has an empty argument pack, and another parameter
	pack has no argument pack at all, use the PACK_EXPANSION_EXTRA
	mechanism.  Update comments.
---
 gcc/cp/pt.c                              | 58 +++++++++++++++++++++--------
 gcc/testsuite/g++.dg/cpp0x/variadic143.C | 63 ++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic143.C
Jason Merrill - May 15, 2013, 3:37 p.m.
On 05/15/2013 10:33 AM, Dodji Seketeli wrote:
> So let's rule out the particular case of a zero argument pack
> length.

I don't think there's anything special about zero length; if 
has_empty_arg is true and parm_packs is non-null, we want to return 
true.  Does that make sense to you?

Jason

Patch

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 7430289..d31af55 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -9158,31 +9158,57 @@  use_pack_expansion_extra_args_p (tree parm_packs,
   if (parm_packs == NULL_TREE)
     return false;
 
+  /* The general reasoning is the following:
+
+     If one pack has an expansion and another pack has a normal
+     argument or if one pack has an empty argument and another one
+     hasn't then tsubst_pack_expansion cannot perform the substitution
+     and need to fall back on the PACK_EXPANSION_EXTRA mechanism so we
+     must return TRUE.  */
+
   bool has_expansion_arg = false;
-  for (int i = 0 ; i < arg_pack_len; ++i)
-    {
-      bool has_non_expansion_arg = false;
+  if (arg_pack_len == 0)
+    {
+      /* So let's rule out the particular case of a zero argument pack
+	 length.  It means either there is parameter pack that has an
+	 empty argument argument pack, or there are no argument packs
+	 at all.  In the former case, if there is also at least a
+	 parameter pack that has no argument pack, then we must return
+	 true.*/
       for (tree parm_pack = parm_packs;
 	   parm_pack;
 	   parm_pack = TREE_CHAIN (parm_pack))
 	{
 	  tree arg = TREE_VALUE (parm_pack);
 
-	  if (argument_pack_element_is_expansion_p (arg, i))
-	    has_expansion_arg = true;
-	  else
-	    has_non_expansion_arg = true;
+	  if (arg
+	      && ARGUMENT_PACK_P (arg)
+	      && TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg)) == 0
+	      && has_empty_arg)
+	    return true;
 	}
-
-      /* If one pack has an expansion and another pack has a normal
-	 argument or if one pack has an empty argument another one
-	 hasn't then tsubst_pack_expansion cannot perform the
-	 substitution and need to fall back on the
-	 PACK_EXPANSION_EXTRA mechanism.  */
-      if ((has_expansion_arg && has_non_expansion_arg)
-	  || (has_empty_arg && (has_expansion_arg || has_non_expansion_arg)))
-	return true;
     }
+  else
+    /* So let's handle the general case.  */
+    for (int i = 0 ; i < arg_pack_len; ++i)
+      {
+	bool has_non_expansion_arg = false;
+	for (tree parm_pack = parm_packs;
+	     parm_pack;
+	     parm_pack = TREE_CHAIN (parm_pack))
+	  {
+	    tree arg = TREE_VALUE (parm_pack);
+
+	    if (argument_pack_element_is_expansion_p (arg, i))
+	      has_expansion_arg = true;
+	    else
+	      has_non_expansion_arg = true;
+	  }
+
+	if ((has_expansion_arg && has_non_expansion_arg)
+	    || (has_empty_arg && (has_expansion_arg || has_non_expansion_arg)))
+	  return true;
+      }
   return false;
 }
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic143.C b/gcc/testsuite/g++.dg/cpp0x/variadic143.C
new file mode 100644
index 0000000..7737b4c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic143.C
@@ -0,0 +1,63 @@ 
+// PR c++/56782
+// { dg-options -std=c++0x }
+
+template<class T>
+T&& declval();
+
+struct is_convertible_impl {
+  template<class T>
+  static void sink(T);
+
+  template<class T, class U, class = decltype(sink<U>(declval<T>()))>
+  static auto test(int) -> char;
+
+  template<class, class>
+  static auto test(...) -> char(&)[2];
+};
+
+template<class T, class U>
+struct is_convertible : is_convertible_impl
+{
+  static const bool value = sizeof(test<T, U>(0)) == 1;
+};
+
+template<bool, class>
+struct enable_if {};
+
+template<class T>
+struct enable_if<true, T> { typedef T type; };
+
+template<bool, class If, class Else>
+struct conditional { typedef If type; };
+
+template<class If, class Else>
+struct conditional<false, If, Else> { typedef Else type; };
+
+template<class...>
+struct and_;
+
+template<>
+struct and_<>
+{
+  static const bool value = true;
+};
+
+template<class P>
+struct and_<P> : P
+{
+};
+
+template<class P1, class P2>
+struct and_<P1, P2> : conditional<P1::value, P2, P1>::type
+{
+};
+
+template<class... T>
+struct Tuple {
+  template<class... U,
+	   class = typename enable_if<and_<is_convertible<U, T>... >::value, int>::type
+	   >
+  Tuple(U&&...){}
+};
+
+static_assert(is_convertible<Tuple<>, Tuple<>>::value, "Ouch"); //#1