diff mbox series

c++: alias template and empty parameter packs [PR104008]

Message ID 20220316211830.373632-1-polacek@redhat.com
State New
Headers show
Series c++: alias template and empty parameter packs [PR104008] | expand

Commit Message

Marek Polacek March 16, 2022, 9:18 p.m. UTC
Zero-length pack expansions are treated as if no list were provided
at all, that is, with

  template<typename...> struct S { };
  template<typename T, typename... Ts>
  void g() {
    S<std::is_same<T, Ts>...>;
  }

g<int> will result in S<>.  In the following test we have something
similar:

  template <typename T, typename... Ts>
  using IsOneOf = disjunction<is_same<T, Ts>...>;

and then we have "IsOneOf<OtherHolders>..." where OtherHolders is an
empty pack.  Since r11-7931, we strip_typedefs in TYPE_PACK_EXPANSION.
In this test that results in "IsOneOf<OtherHolders>" being turned into
"disjunction<>".  So the whole expansion is now "disjunction<>...".  But
then we error in make_pack_expansion because find_parameter_packs_r won't
find the pack OtherHolders.

We strip the alias template because dependent_alias_template_spec_p says
it's not dependent.  It it not dependent because this alias is not
TEMPLATE_DECL_COMPLEX_ALIAS_P.  My understanding is that currently we
consider an alias complex if it

1) expands a pack from the enclosing class, as in

    template<template<typename... U> typename... TT>
    struct S {
      template<typename... Args>
      using X = P<TT<Args...>...>;
    };

   where the alias expands TT; or

2) the expansion does *not* name all the template parameters, as in

    template<typename...> struct R;
    template<typename T, typename... Ts>
    using U = R<X<Ts>...>;

   where T is not named in the expansion.

But IsOneOf is neither.  And it can't know how it's going to be used.
Therefore I think we cannot make it complex (and in turn dependent) to fix
this bug.

After much gnashing of teeth, I think we simply want to avoid stripping
the alias if the new pattern doesn't have any parameter packs to expand.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/11?

	PR c++/104008

gcc/cp/ChangeLog:

	* tree.cc (strip_typedefs): Don't strip an alias template when
	doing so would result in losing a parameter pack.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/variadic-alias3.C: New test.
	* g++.dg/cpp0x/variadic-alias4.C: New test.
---
 gcc/cp/tree.cc                               | 13 +++++-
 gcc/testsuite/g++.dg/cpp0x/variadic-alias3.C | 45 ++++++++++++++++++
 gcc/testsuite/g++.dg/cpp0x/variadic-alias4.C | 48 ++++++++++++++++++++
 3 files changed, 105 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic-alias3.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic-alias4.C


base-commit: 7fd6e36ea9aa8575841ff1da08b4aebc0298abe2

Comments

Jason Merrill March 18, 2022, 4:42 p.m. UTC | #1
On 3/16/22 17:18, Marek Polacek wrote:
> Zero-length pack expansions are treated as if no list were provided
> at all, that is, with
> 
>    template<typename...> struct S { };
>    template<typename T, typename... Ts>
>    void g() {
>      S<std::is_same<T, Ts>...>;
>    }
> 
> g<int> will result in S<>.  In the following test we have something
> similar:
> 
>    template <typename T, typename... Ts>
>    using IsOneOf = disjunction<is_same<T, Ts>...>;
> 
> and then we have "IsOneOf<OtherHolders>..." where OtherHolders is an
> empty pack.  Since r11-7931, we strip_typedefs in TYPE_PACK_EXPANSION.
> In this test that results in "IsOneOf<OtherHolders>" being turned into
> "disjunction<>".  So the whole expansion is now "disjunction<>...".  But
> then we error in make_pack_expansion because find_parameter_packs_r won't
> find the pack OtherHolders.
> 
> We strip the alias template because dependent_alias_template_spec_p says
> it's not dependent.  It it not dependent because this alias is not
> TEMPLATE_DECL_COMPLEX_ALIAS_P.  My understanding is that currently we
> consider an alias complex if it
> 
> 1) expands a pack from the enclosing class, as in
> 
>      template<template<typename... U> typename... TT>
>      struct S {
>        template<typename... Args>
>        using X = P<TT<Args...>...>;
>      };
> 
>     where the alias expands TT; or
> 
> 2) the expansion does *not* name all the template parameters, as in
> 
>      template<typename...> struct R;
>      template<typename T, typename... Ts>
>      using U = R<X<Ts>...>;
> 
>     where T is not named in the expansion.
> 
> But IsOneOf is neither.  And it can't know how it's going to be used.
> Therefore I think we cannot make it complex (and in turn dependent) to fix
> this bug.
> 
> After much gnashing of teeth, I think we simply want to avoid stripping
> the alias if the new pattern doesn't have any parameter packs to expand.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/11?
> 
> 	PR c++/104008
> 
> gcc/cp/ChangeLog:
> 
> 	* tree.cc (strip_typedefs): Don't strip an alias template when
> 	doing so would result in losing a parameter pack.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/variadic-alias3.C: New test.
> 	* g++.dg/cpp0x/variadic-alias4.C: New test.
> ---
>   gcc/cp/tree.cc                               | 13 +++++-
>   gcc/testsuite/g++.dg/cpp0x/variadic-alias3.C | 45 ++++++++++++++++++
>   gcc/testsuite/g++.dg/cpp0x/variadic-alias4.C | 48 ++++++++++++++++++++
>   3 files changed, 105 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic-alias3.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic-alias4.C
> 
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 6e9be713c51..eb59e56610b 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -1778,7 +1778,18 @@ strip_typedefs (tree t, bool *remove_attributes, unsigned int flags)
>   	if (TYPE_P (pat))
>   	  {
>   	    type = strip_typedefs (pat, remove_attributes, flags);
> -	    if (type != pat)
> +	    /* Empty packs can thwart our efforts here.  Consider
> +
> +		template <typename T, typename... Ts>
> +		using IsOneOf = disjunction<is_same<T, Ts>...>;
> +
> +	      where IsOneOf seemingly uses all of its template parameters in
> +	      its expansion (and does not expand a pack from the enclosing
> +	      class), so the alias is not marked as complex.  However, it may
> +	      be used as in "IsOneOf<Ts>", where Ts is an empty parameter pack,
> +	      and stripping it down into "disjunction<>" here would exclude the
> +	      Ts pack, resulting in an error.  */
> +	    if (type != pat && uses_parameter_packs (type))

I wonder if we want to verify that the result of uses_parameter_packs 
matches PACK_EXPANSION_PARAMETER_PACKS, not just that it's non-null? 
But I can't think how to write a testcase where those two questions have 
different answers.

The patch is OK, thanks.

>   	      {
>   		result = copy_node (t);
>   		PACK_EXPANSION_PATTERN (result) = type;
> diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic-alias3.C b/gcc/testsuite/g++.dg/cpp0x/variadic-alias3.C
> new file mode 100644
> index 00000000000..6b6dd9f4c85
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/variadic-alias3.C
> @@ -0,0 +1,45 @@
> +// PR c++/104008
> +// { dg-do compile { target c++11 } }
> +
> +template <typename...> struct conjunction;
> +template <typename...> struct disjunction;
> +template <typename, typename> struct is_same;
> +template <bool> struct enable_if;
> +template <bool _Cond> using enable_if_t = typename enable_if<_Cond>::type;
> +struct B;
> +struct __uniq_ptr_impl {
> +  struct _Ptr {
> +    using type = B *;
> +  };
> +  using pointer = _Ptr::type;
> +};
> +struct unique_ptr {
> +  using pointer = __uniq_ptr_impl::pointer;
> +  unique_ptr(pointer);
> +};
> +template <typename T, typename... Ts>
> +using IsOneOf = disjunction<is_same<T, Ts>...>;
> +
> +template <typename...> struct any_badge;
> +
> +struct badge {
> +  badge(any_badge<>);
> +  badge();
> +};
> +
> +template <typename...> struct any_badge {
> +  template <typename... OtherHolders,
> +            enable_if_t<conjunction<IsOneOf<OtherHolders>...>::value>>
> +  any_badge();
> +};
> +
> +template <typename, typename... _Args> unique_ptr make_unique(_Args... __args);
> +
> +struct B {
> +  B(badge);
> +  unique_ptr b_ = make_unique<B>(badge{});
> +};
> +
> +template <typename, typename... _Args> unique_ptr make_unique(_Args... __args) {
> +  return new B(__args...);
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic-alias4.C b/gcc/testsuite/g++.dg/cpp0x/variadic-alias4.C
> new file mode 100644
> index 00000000000..896a4725627
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/variadic-alias4.C
> @@ -0,0 +1,48 @@
> +// PR c++/104008
> +// { dg-do compile { target c++11 } }
> +// Differs from variadic-alias3.C only in the pattern of a pack expansion
> +// in line 34.  But it's important to check that we also deal with more
> +// complex patterns.
> +
> +template <typename...> struct conjunction;
> +template <typename...> struct disjunction;
> +template <typename, typename> struct is_same;
> +template <bool> struct enable_if;
> +template <bool _Cond> using enable_if_t = typename enable_if<_Cond>::type;
> +struct B;
> +struct __uniq_ptr_impl {
> +  struct _Ptr {
> +    using type = B *;
> +  };
> +  using pointer = _Ptr::type;
> +};
> +struct unique_ptr {
> +  using pointer = __uniq_ptr_impl::pointer;
> +  unique_ptr(pointer);
> +};
> +template <typename T, typename... Ts>
> +using IsOneOf = disjunction<is_same<T, Ts>...>;
> +
> +template <typename...> struct any_badge;
> +
> +struct badge {
> +  badge(any_badge<>);
> +  badge();
> +};
> +
> +template <typename...> struct any_badge {
> +  template <typename... OtherHolders,
> +            enable_if_t<conjunction<IsOneOf<OtherHolders>&...>::value>>
> +  any_badge();
> +};
> +
> +template <typename, typename... _Args> unique_ptr make_unique(_Args... __args);
> +
> +struct B {
> +  B(badge);
> +  unique_ptr b_ = make_unique<B>(badge{});
> +};
> +
> +template <typename, typename... _Args> unique_ptr make_unique(_Args... __args) {
> +  return new B(__args...);
> +}
> 
> base-commit: 7fd6e36ea9aa8575841ff1da08b4aebc0298abe2
diff mbox series

Patch

diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 6e9be713c51..eb59e56610b 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -1778,7 +1778,18 @@  strip_typedefs (tree t, bool *remove_attributes, unsigned int flags)
 	if (TYPE_P (pat))
 	  {
 	    type = strip_typedefs (pat, remove_attributes, flags);
-	    if (type != pat)
+	    /* Empty packs can thwart our efforts here.  Consider
+
+		template <typename T, typename... Ts>
+		using IsOneOf = disjunction<is_same<T, Ts>...>;
+
+	      where IsOneOf seemingly uses all of its template parameters in
+	      its expansion (and does not expand a pack from the enclosing
+	      class), so the alias is not marked as complex.  However, it may
+	      be used as in "IsOneOf<Ts>", where Ts is an empty parameter pack,
+	      and stripping it down into "disjunction<>" here would exclude the
+	      Ts pack, resulting in an error.  */
+	    if (type != pat && uses_parameter_packs (type))
 	      {
 		result = copy_node (t);
 		PACK_EXPANSION_PATTERN (result) = type;
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic-alias3.C b/gcc/testsuite/g++.dg/cpp0x/variadic-alias3.C
new file mode 100644
index 00000000000..6b6dd9f4c85
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic-alias3.C
@@ -0,0 +1,45 @@ 
+// PR c++/104008
+// { dg-do compile { target c++11 } }
+
+template <typename...> struct conjunction;
+template <typename...> struct disjunction;
+template <typename, typename> struct is_same;
+template <bool> struct enable_if;
+template <bool _Cond> using enable_if_t = typename enable_if<_Cond>::type;
+struct B;
+struct __uniq_ptr_impl {
+  struct _Ptr {
+    using type = B *;
+  };
+  using pointer = _Ptr::type;
+};
+struct unique_ptr {
+  using pointer = __uniq_ptr_impl::pointer;
+  unique_ptr(pointer);
+};
+template <typename T, typename... Ts>
+using IsOneOf = disjunction<is_same<T, Ts>...>;
+
+template <typename...> struct any_badge;
+
+struct badge {
+  badge(any_badge<>);
+  badge();
+};
+
+template <typename...> struct any_badge {
+  template <typename... OtherHolders,
+            enable_if_t<conjunction<IsOneOf<OtherHolders>...>::value>>
+  any_badge();
+};
+
+template <typename, typename... _Args> unique_ptr make_unique(_Args... __args);
+
+struct B {
+  B(badge);
+  unique_ptr b_ = make_unique<B>(badge{});
+};
+
+template <typename, typename... _Args> unique_ptr make_unique(_Args... __args) {
+  return new B(__args...);
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic-alias4.C b/gcc/testsuite/g++.dg/cpp0x/variadic-alias4.C
new file mode 100644
index 00000000000..896a4725627
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic-alias4.C
@@ -0,0 +1,48 @@ 
+// PR c++/104008
+// { dg-do compile { target c++11 } }
+// Differs from variadic-alias3.C only in the pattern of a pack expansion
+// in line 34.  But it's important to check that we also deal with more
+// complex patterns.
+
+template <typename...> struct conjunction;
+template <typename...> struct disjunction;
+template <typename, typename> struct is_same;
+template <bool> struct enable_if;
+template <bool _Cond> using enable_if_t = typename enable_if<_Cond>::type;
+struct B;
+struct __uniq_ptr_impl {
+  struct _Ptr {
+    using type = B *;
+  };
+  using pointer = _Ptr::type;
+};
+struct unique_ptr {
+  using pointer = __uniq_ptr_impl::pointer;
+  unique_ptr(pointer);
+};
+template <typename T, typename... Ts>
+using IsOneOf = disjunction<is_same<T, Ts>...>;
+
+template <typename...> struct any_badge;
+
+struct badge {
+  badge(any_badge<>);
+  badge();
+};
+
+template <typename...> struct any_badge {
+  template <typename... OtherHolders,
+            enable_if_t<conjunction<IsOneOf<OtherHolders>&...>::value>>
+  any_badge();
+};
+
+template <typename, typename... _Args> unique_ptr make_unique(_Args... __args);
+
+struct B {
+  B(badge);
+  unique_ptr b_ = make_unique<B>(badge{});
+};
+
+template <typename, typename... _Args> unique_ptr make_unique(_Args... __args) {
+  return new B(__args...);
+}