On Thu, 25 Jul 2024, Patrick Palka wrote:

> On Mon, 22 Jul 2024, Jason Merrill wrote:
> 
> > On 7/19/24 10:30 AM, Patrick Palka wrote:
> > > On Thu, 18 Jul 2024, Jason Merrill wrote:
> > > 
> > > > On 7/18/24 12:45 PM, Patrick Palka wrote:
> > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does thi look
> > > > > OK for trunk/14?
> > > > > 
> > > > > -- >8 --
> > > > > 
> > > > > As a followup of r15-2047-g7954bb4fcb6fa8, we also need to consider
> > > > > dependent attributes when recursing into a non-template alias that 
> > > > > names
> > > > > a dependent alias template specialization (and so STF_STRIP_DEPENDENT
> > > > > is set), otherwise in the first testcase below we undesirably strip B
> > > > > all the way to T instead of to A<T>.
> > > > > 
> > > > > We also need to move the typedef recursion case of strip_typedefs up 
> > > > > to
> > > > > get checked before the compound type recursion cases.  Otherwise for C
> > > > > below (which ultimately aliases T*) we end up stripping it to T* 
> > > > > instead
> > > > > of to A<T*> because the POINTER_TYPE recursion dominates the typedef
> > > > > recursion.  It also means we issue an unexpected extra error in the
> > > > > third testcase below.
> > > > > 
> > > > > Ideally we would also want to consider dependent attributes on
> > > > > non-template aliases, so that we accept the second testcase below, but
> > > > > making that work correctly would require broader changes to e.g.
> > > > > spec_hasher which currently assumes all non-template aliases are
> > > > > stripped and hence it'd conflate the dependent specializations A<T>
> > > > > and A<B> even if we didn't strip B.
> > > > 
> > > > Wouldn't that just be a matter of changing structural_comptypes to
> > > > consider
> > > > dependent attributes as well as dependent specializations?
> > > 
> > > Pretty much, it seems.  ISTM we should check dependent attributes even
> > > when !comparing_dependent_aliases since they affect type identity rather
> > > than just SFINAE behavior.
> > > 
> > > > 
> > > > Or better, adding attributes to dependent_alias_template_spec_p (and
> > > > changing
> > > > its name)?  It seems like other callers would also benefit from that
> > > > change.
> > > 
> > > I ended up adding a new predicate opaque_alias_p separate from
> > > dependent_alias_template_spec_p since ISTM we need to call it from
> > > there and from alias_template_specialization_p to avoid looking through
> > > such aliases.
> > 
> > Sounds good, but I think let's add the word "dependent" to the name of the 
> > new
> > function.
> 
> Done.
> 
> > 
> > > So opaque_alias_p checks for type identity of an alias, whereas
> > > dependent_alias_template_spec_p more broadly checks for SFINAE identity.
> > > 
> > > Something like the following (as an incremental patch on top of the
> > > previous one, to consider separately for backportings since it's riskier):
> > > 
> > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > > index 0620c8c023a..4d4a5cef92c 100644
> > > --- a/gcc/cp/pt.cc
> > > +++ b/gcc/cp/pt.cc
> > > @@ -6508,6 +6508,19 @@ alias_type_or_template_p (tree t)
> > >             || DECL_ALIAS_TEMPLATE_P (t));
> > >   }
> > >   +/* Return true if substituting into T would yield a different type than
> > > +   substituting into its expansion.  */
> > 
> > Please discuss when to use this vs dependent_alias_template_spec_p (both 
> > here
> > and there).  Maybe just to say that any place that checks one probably also
> > wants to check the other.
> > 
> > Other places that use d_a_t_s_p and seem to need adjusting:
> > dependent_type_p_r, any_dependent_arguments_need_structural_equality_p,
> > alias_ctad_tweaks.
> 
> For dependent_type_p_r, I think we can get rid of the existing d_a_t_s_p
> check if we manually set TYPE_DEPENDENT_P/_VALID to true at parse time
> like we do at instantiation time from instantiate_alias_template.
> 
> For any_dependent_arguments_need_structural_equality_p, those checks
> can't be removed but we can relax nt_transparent to nt_opaque since we
> know the template arguments have already gone through strip_typedefs.
> 
> For alias_ctad_tweaks I think we can replace the d_a_t_s_p with
> template_args_equal which has the desired comparing_dependent_aliases
> behavior.
> 
> After the above, it seems no relevant user of d_a_t_s_p survives that
> passes nt_transparent instead of nt_opaque, and so we can avoid adding
> a transparent_typedefs flag to the new predicate.
> 
> > 
> > > +bool
> > > +opaque_alias_p (const_tree t)
> > > +{
> > > +  return (TYPE_P (t)
> > > +   && typedef_variant_p (t)
> > > +   && uses_template_parms (DECL_ORIGINAL_TYPE (TYPE_NAME (t)))
> > 
> > Checking this seems wrong; a vector of dependent size seems opaque even if
> > it's a vector of int.
> 
> Makes sense, fixed.  I added a testcase variant too.

I forgot to mention, the related PRs PR83997 etc seem to be about
propagating/respecting attributes attached to the aliased type rather
than to the alias declaration.

I tried making dependent_opaque_alias_p also consider TYPE_ATTRIBUTES
alongside DECL_ATTRIBUTES, but that wasn't enough becuase we don't
actually call apply_late_template_attributes on the TYPE_ATTRIBUTES at
alias instantiation time and we end up ignoring them.  So fixing that
seems to be a mostly orthogonal issue (and perhaps relatively low
priority since it works to just move the attribute to the decl).

> 
> -- >8 --
> 
> Subject: [PATCH 2/1] c++: non-template alias with dependent attributes 
> [PR115897]
> 
>       PR c++/115897
> 
> gcc/cp/ChangeLog:
> 
>       * cp-tree.h (dependent_opaque_alias_p): Declare.
>       * pt.cc (push_template_decl): Manually mark a dependent opaque
>       alias or dependent alias template specialization as dependent,
>       and use structural equality for them.
>       * pt.cc (dependent_opaque_alias_p): Define.
>       (alias_template_specialization_p): Don't look through an
>       opaque alias.
>       (complex_alias_template_p): Use dependent_opaque_alias_p instead of
>       any_dependent_template_arguments_p directly.
>       (dependent_alias_template_spec_p): Don't look through an
>       opaque alias.
>       (get_underlying_template): Use dependent_opaque_alias_p instead of
>       any_dependent_template_arguments_p.
>       (instantiate_alias_template): Add comment mentioning similar
>       behavior in push_template_decl above.
>       (dependent_type_p_r): Remove dependent_alias_template_spec_p check.
>       (any_template_arguments_need_structural_equality_p): Return true
>       for a dependent opaque alias argument.
>       (alias_ctad_tweaks): Use template_args_equal instead of same_type_p
>       followed by dependent_alias_template_spec_p.
>       * tree.cc (strip_typedefs): Don't strip an opaque alias.
>       * typeck.cc (structural_comptypes): Compare declaration attributes
>       for an opaque alias.
> 
> gcc/testsuite/ChangeLog:
> 
>       * g++.dg/cpp0x/alias-decl-79.C: Remove xfails.
>       * g++.dg/cpp0x/alias-decl-79a.C: New test.
> ---
>  gcc/cp/cp-tree.h                            |  1 +
>  gcc/cp/pt.cc                                | 55 +++++++++++++++------
>  gcc/cp/tree.cc                              |  7 +--
>  gcc/cp/typeck.cc                            | 17 +++++--
>  gcc/testsuite/g++.dg/cpp0x/alias-decl-79.C  | 16 +++---
>  gcc/testsuite/g++.dg/cpp0x/alias-decl-79a.C | 41 +++++++++++++++
>  6 files changed, 106 insertions(+), 31 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-79a.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 7d50aac4b6b..0de181ae84d 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7619,6 +7619,7 @@ extern tree instantiate_non_dependent_or_null   (tree);
>  extern bool variable_template_specialization_p  (tree);
>  extern bool alias_type_or_template_p            (tree);
>  enum { nt_opaque = false, nt_transparent = true };
> +extern bool dependent_opaque_alias_p                 (const_tree);
>  extern tree alias_template_specialization_p     (const_tree, bool);
>  extern tree dependent_alias_template_spec_p     (const_tree, bool);
>  extern tree get_template_parm_object         (tree expr, tree mangle);
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index e102e3ea490..4da3bb49ccf 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -6271,6 +6271,18 @@ push_template_decl (tree decl, bool is_friend)
>       }
>      }
>  
> +  if (is_typedef_decl (decl)
> +      && (dependent_opaque_alias_p (TREE_TYPE (decl))
> +       || dependent_alias_template_spec_p (TREE_TYPE (decl), nt_opaque)))
> +    {
> +      /* Manually mark such aliases as dependent so that dependent_type_p_r
> +      doesn't have to check these predicates.  */
> +      TYPE_DEPENDENT_P_VALID (TREE_TYPE (decl)) = true;
> +      TYPE_DEPENDENT_P (TREE_TYPE (decl)) = true;
> +      /* The identity of such aliases is hairy; see structural_comptypes.  */
> +      SET_TYPE_STRUCTURAL_EQUALITY (TREE_TYPE (decl));
> +    }
> +
>    if (flag_implicit_templates
>        && !is_friend
>        && TREE_PUBLIC (decl)
> @@ -6513,6 +6525,19 @@ alias_type_or_template_p (tree t)
>         || DECL_ALIAS_TEMPLATE_P (t));
>  }
>  
> +/* Return true if substituting into T would yield a different type than
> +   substituting into its expansion.  Checking this predicate is usually
> +   done in tandem with dependent_alias_template_spec_p.  */
> +
> +bool
> +dependent_opaque_alias_p (const_tree t)
> +{
> +  return (TYPE_P (t)
> +       && typedef_variant_p (t)
> +       && any_dependent_type_attributes_p (DECL_ATTRIBUTES
> +                                           (TYPE_NAME (t))));
> +}
> +
>  /* If T is a specialization of an alias template, return it; otherwise return
>     NULL_TREE.  If TRANSPARENT_TYPEDEFS is true, look through other aliases.  
> */
>  
> @@ -6530,7 +6555,7 @@ alias_template_specialization_p (const_tree t,
>        if (tree tinfo = TYPE_ALIAS_TEMPLATE_INFO (t))
>       if (PRIMARY_TEMPLATE_P (TI_TEMPLATE (tinfo)))
>         return CONST_CAST_TREE (t);
> -      if (transparent_typedefs)
> +      if (transparent_typedefs && !dependent_opaque_alias_p (t))
>       return alias_template_specialization_p (DECL_ORIGINAL_TYPE
>                                               (TYPE_NAME (t)),
>                                               transparent_typedefs);
> @@ -6635,8 +6660,7 @@ complex_alias_template_p (const_tree tmpl, tree 
> *seen_out)
>      return true;
>  
>    /* An alias with dependent type attributes is complex.  */
> -  if (any_dependent_type_attributes_p (DECL_ATTRIBUTES
> -                                    (DECL_TEMPLATE_RESULT (tmpl))))
> +  if (dependent_opaque_alias_p (TREE_TYPE (tmpl)))
>      return true;
>  
>    if (!complex_alias_tmpl_info)
> @@ -6687,7 +6711,10 @@ complex_alias_template_p (const_tree tmpl, tree 
> *seen_out)
>  /* If T is a specialization of a complex alias template with a dependent
>     argument for an unused template parameter, return it; otherwise return
>     NULL_TREE.  If T is a typedef to such a specialization, return the
> -   specialization.  */
> +   specialization.  Checking this predicate is usually done in tandem
> +   with dependent_opaque_alias_p.  Whereas dependent_opaque_alias_p checks
> +   type equivalence of an alias vs its expansion, this predicate more
> +   broadly checks SFINAE equivalence.  */
>  
>  tree
>  dependent_alias_template_spec_p (const_tree t, bool transparent_typedefs)
> @@ -6723,7 +6750,7 @@ dependent_alias_template_spec_p (const_tree t, bool 
> transparent_typedefs)
>       }
>      }
>  
> -  if (transparent_typedefs)
> +  if (transparent_typedefs && !dependent_opaque_alias_p (t))
>      {
>        tree utype = DECL_ORIGINAL_TYPE (TYPE_NAME (t));
>        return dependent_alias_template_spec_p (utype, transparent_typedefs);
> @@ -6792,8 +6819,7 @@ get_underlying_template (tree tmpl)
>       break;
>  
>        /* If TMPL adds dependent type attributes, it isn't equivalent.  */
> -      if (any_dependent_type_attributes_p (DECL_ATTRIBUTES
> -                                        (DECL_TEMPLATE_RESULT (tmpl))))
> +      if (dependent_opaque_alias_p (TREE_TYPE (tmpl)))
>       break;
>  
>        /* Alias is equivalent.  Strip it and repeat.  */
> @@ -22278,6 +22304,7 @@ instantiate_alias_template (tree tmpl, tree args, 
> tsubst_flags_t complain)
>  
>    if (tree d = dependent_alias_template_spec_p (TREE_TYPE (r), nt_opaque))
>      {
> +      /* Note this is also done at parse time from push_template_decl.  */
>        /* An alias template specialization can be dependent
>        even if its underlying type is not.  */
>        TYPE_DEPENDENT_P (d) = true;
> @@ -27906,11 +27933,6 @@ dependent_type_p_r (tree type)
>    if (TREE_CODE (type) == TYPENAME_TYPE)
>      return true;
>  
> -  /* An alias template specialization can be dependent even if the
> -     resulting type is not.  */
> -  if (dependent_alias_template_spec_p (type, nt_transparent))
> -    return true;
> -
>    /* -- a cv-qualified type where the cv-unqualified type is
>       dependent.
>       No code is necessary for this bullet; the code below handles
> @@ -29018,7 +29040,8 @@ any_template_arguments_need_structural_equality_p 
> (tree args)
>               return true;
>             else if (TYPE_P (arg)
>                      && TYPE_STRUCTURAL_EQUALITY_P (arg)
> -                    && dependent_alias_template_spec_p (arg, nt_transparent))
> +                    && (dependent_alias_template_spec_p (arg, nt_opaque)
> +                        || dependent_opaque_alias_p (arg)))
>               /* Require structural equality for specializations written
>                  in terms of a dependent alias template specialization.  */
>               return true;
> @@ -30418,9 +30441,9 @@ alias_ctad_tweaks (tree tmpl, tree uguides)
>            A with the same template arguments.  */
>         ret = TREE_TYPE (TREE_TYPE (fprime));
>         if (ctad_kind == alias
> -           && (!same_type_p (atype, ret)
> -               /* FIXME this should mean they don't compare as equivalent. */
> -               || dependent_alias_template_spec_p (atype, nt_opaque)))
> +           /* Use template_args_equal instead of same_type_p to get the
> +              comparing_dependent_aliases behavior.  */
> +           && !template_args_equal (atype, ret))
>           {
>             tree same = finish_trait_expr (loc, CPTK_IS_DEDUCIBLE, tmpl, ret);
>             ci = append_constraint (ci, same);
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index f2001ace6db..31ecbb1ac79 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -1613,12 +1613,13 @@ strip_typedefs (tree t, bool *remove_attributes /* = 
> NULL */,
>         && !user_facing_original_type_p (t))
>       return t;
>  
> +      if (dependent_opaque_alias_p (t))
> +     return t;
> +
>        if (alias_template_specialization_p (t, nt_opaque))
>       {
>         if (dependent_alias_template_spec_p (t, nt_opaque)
> -           && (!(flags & STF_STRIP_DEPENDENT)
> -               || any_dependent_type_attributes_p (DECL_ATTRIBUTES
> -                                                   (TYPE_NAME (t)))))
> +           && !(flags & STF_STRIP_DEPENDENT))
>           /* DR 1558: However, if the template-id is dependent, subsequent
>              template argument substitution still applies to the template-id. 
>  */
>           return t;
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 8df8b871676..f26b5b2a1f4 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -1658,8 +1658,17 @@ structural_comptypes (tree t1, tree t2, int strict)
>      return false;
>  
>   check_alias:
> -  if (comparing_dependent_aliases)
> -    {
> +  if (comparing_dependent_aliases
> +      && (typedef_variant_p (t1) || typedef_variant_p (t2)))
> +    {
> +      tree dep1 = dependent_opaque_alias_p (t1) ? t1 : NULL_TREE;
> +      tree dep2 = dependent_opaque_alias_p (t2) ? t2 : NULL_TREE;
> +      if ((dep1 || dep2)
> +       && (!(dep1 && dep2)
> +           || !comp_type_attributes (DECL_ATTRIBUTES (TYPE_NAME (dep1)),
> +                                     DECL_ATTRIBUTES (TYPE_NAME (dep2)))))
> +     return false;
> +
>        /* Don't treat an alias template specialization with dependent
>        arguments as equivalent to its underlying type when used as a
>        template argument; we need them to be distinct so that we
> @@ -1667,8 +1676,8 @@ structural_comptypes (tree t1, tree t2, int strict)
>        time.  And aliases can't be equivalent without being ==, so
>        we don't need to look any deeper.  */
>        ++processing_template_decl;
> -      tree dep1 = dependent_alias_template_spec_p (t1, nt_transparent);
> -      tree dep2 = dependent_alias_template_spec_p (t2, nt_transparent);
> +      dep1 = dependent_alias_template_spec_p (t1, nt_transparent);
> +      dep2 = dependent_alias_template_spec_p (t2, nt_transparent);
>        --processing_template_decl;
>        if ((dep1 || dep2) && dep1 != dep2)
>       return false;
> diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-79.C 
> b/gcc/testsuite/g++.dg/cpp0x/alias-decl-79.C
> index e0f07475cc1..58436f907ef 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/alias-decl-79.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-79.C
> @@ -14,22 +14,22 @@ template<class T> struct A;
>  template<class T>
>  void f() {
>    using B [[gnu::vector_size(16)]] = T;
> -  static_assert(!is_same<T, B>::value, "");        // { dg-bogus "" "" { 
> xfail *-*-* } }
> -  static_assert(!is_same<A<T>, A<B>>::value, "");  // { dg-bogus "" "" { 
> xfail *-*-* } }
> +  static_assert(!is_same<T, B>::value, "");
> +  static_assert(!is_same<A<T>, A<B>>::value, "");
>  #if __cpp_variable_templates
> -  static_assert(!is_same_v<T, B>, "");             // { dg-bogus "" "" { 
> xfail c++14 } }
> -  static_assert(!is_same_v<A<T>, A<B>>, "");       // { dg-bogus "" "" { 
> xfail c++14 } }
> +  static_assert(!is_same_v<T, B>, "");
> +  static_assert(!is_same_v<A<T>, A<B>>, "");
>  #endif
>  };
>  
>  template<class T>
>  void g() {
>    using C [[gnu::vector_size(16)]] = T*;
> -  static_assert(!is_same<T*, C>::value, "");       // { dg-bogus "" "" { 
> xfail *-*-* } }
> -  static_assert(!is_same<A<T*>, A<C>>::value, ""); // { dg-bogus "" "" { 
> xfail *-*-* } }
> +  static_assert(!is_same<T*, C>::value, "");
> +  static_assert(!is_same<A<T*>, A<C>>::value, "");
>  #if __cpp_variable_templates
> -  static_assert(!is_same_v<T*, C>, "");            // { dg-bogus "" "" { 
> xfail c++14 } }
> -  static_assert(!is_same_v<A<T*>, A<C>>, "");      // { dg-bogus "" "" { 
> xfail c++14 } }
> +  static_assert(!is_same_v<T*, C>, "");
> +  static_assert(!is_same_v<A<T*>, A<C>>, "");
>  #endif
>  };
>  
> diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-79a.C 
> b/gcc/testsuite/g++.dg/cpp0x/alias-decl-79a.C
> new file mode 100644
> index 00000000000..151b8487e1f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-79a.C
> @@ -0,0 +1,41 @@
> +// A version of alias-decl-79.C where defining-type-id of B and C
> +// are not dependent and instead their vector_size attribute is.
> +// PR c++/115897
> +// { dg-do compile { target c++11 } }
> +
> +template<class T, class U>
> +struct is_same { static constexpr bool value = __is_same(T, U); };
> +
> +#if __cpp_variable_templates
> +template<class T, class U>
> +constexpr bool is_same_v = __is_same(T, U);
> +#endif
> +
> +template<class T> struct A;
> +
> +template<int N>
> +void f() {
> +  using T = float;
> +  using B [[gnu::vector_size(N * sizeof(float))]] = T;
> +  static_assert(!is_same<T, B>::value, "");
> +  static_assert(!is_same<A<T>, A<B>>::value, "");
> +#if __cpp_variable_templates
> +  static_assert(!is_same_v<T, B>, "");
> +  static_assert(!is_same_v<A<T>, A<B>>, "");
> +#endif
> +};
> +
> +template<int N>
> +void g() {
> +  using T = float*;
> +  using C [[gnu::vector_size(N * sizeof(float*))]] = T;
> +  static_assert(!is_same<T*, C>::value, "");
> +  static_assert(!is_same<A<T*>, A<C>>::value, "");
> +#if __cpp_variable_templates
> +  static_assert(!is_same_v<T*, C>, "");
> +  static_assert(!is_same_v<A<T*>, A<C>>, "");
> +#endif
> +};
> +
> +template void f<4>();
> +template void g<4>();
> -- 
> 2.46.0.rc2
> 
> 

Reply via email to