Re: [C++ PATCH] Fix __direct_bases ICE (PR c++/85146, take 2)

[Jason Merrill <jason at redhat dot com>]

On Wed, Apr 4, 2018 at 5:28 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Apr 03, 2018 at 12:23:11PM -0400, Jason Merrill wrote:
>> On Tue, Apr 3, 2018 at 12:06 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>
>> > From N2965 it is unclear to me what should the trait do on classes with
>> > incomplete types, but given that __bases already returns an empty pack,
>> > this patch does the same for __direct_bases.
>> >
>>
>> I think it should be ill-formed, i.e. call complete_type_or_maybe_complain
>> from tsubst_pack_expansion.
>
> calculate_*bases already calls complete_type, so it seems better to me
> to just pass complain down to those and handle it there.
>
> Ok for trunk if it passes bootstrap/regtest?
>
> I've also double-checked that it works with stuff like
> va_list ap;
> B<__typeof (*ap)> b;
> where __va_list_tag is RECORD_TYPE but not CLASS_TYPE_P.
>
> 2018-04-04  Jakub Jelinek  <jakub@redhat.com>
>
>         PR c++/85146
>         * cp-tree.h (calculate_bases, calculate_direct_bases): Add complain
>         argument.
>         * semantics.c (calculate_direct_bases, calculate_bases): Add complain
>         argument.  Use complete_type_or_maybe_complain instead of just
>         complete_type and return an empty vector if it fails.  Formatting
>         fixes.
>         (dfs_calculate_bases_post, calculate_bases_helper): Formatting fixes.
>         * pt.c (tsubst_pack_expansion): Adjust calculate_bases and
>         calculate_direct_bases callers, formatting fixes.
>
>         * g++.dg/ext/bases2.C: Expect extra error diagnostics.
>         * g++.dg/ext/bases3.C: New test.
>
> --- gcc/cp/cp-tree.h.jj 2018-04-03 23:39:16.601665294 +0200
> +++ gcc/cp/cp-tree.h    2018-04-04 10:34:20.451307112 +0200
> @@ -6870,9 +6870,9 @@ extern cp_expr finish_id_expression               (tr
>                                                   location_t);
>  extern tree finish_typeof                      (tree);
>  extern tree finish_underlying_type             (tree);
> -extern tree calculate_bases                     (tree);
> +extern tree calculate_bases                     (tree, tsubst_flags_t);
>  extern tree finish_bases                        (tree, bool);
> -extern tree calculate_direct_bases              (tree);
> +extern tree calculate_direct_bases              (tree, tsubst_flags_t);
>  extern tree finish_offsetof                    (tree, tree, location_t);
>  extern void finish_decl_cleanup                        (tree, tree);
>  extern void finish_eh_cleanup                  (tree);
> --- gcc/cp/semantics.c.jj       2018-04-03 23:39:16.559665288 +0200
> +++ gcc/cp/semantics.c  2018-04-04 11:17:47.455507857 +0200
> @@ -3885,49 +3885,38 @@ finish_underlying_type (tree type)
>  }
>
>  /* Implement the __direct_bases keyword: Return the direct base classes
> -   of type */
> +   of type.  */
>
>  tree
> -calculate_direct_bases (tree type)
> +calculate_direct_bases (tree type, tsubst_flags_t complain)
>  {
> -  vec<tree, va_gc> *vector = make_tree_vector();
> +  vec<tree, va_gc> *vector = make_tree_vector ();
>    tree bases_vec = NULL_TREE;
>    vec<tree, va_gc> *base_binfos;
>    tree binfo;
>    unsigned i;
>
> -  complete_type (type);
> -
> -  if (!NON_UNION_CLASS_TYPE_P (type))
> +  if (!complete_type_or_maybe_complain (type, NULL_TREE, complain)
> +      || !NON_UNION_CLASS_TYPE_P (type))
>      return make_tree_vec (0);
>
>    base_binfos = BINFO_BASE_BINFOS (TYPE_BINFO (type));
>
>    /* Virtual bases are initialized first */
>    for (i = 0; base_binfos->iterate (i, &binfo); i++)
> -    {
> -      if (BINFO_VIRTUAL_P (binfo))
> -       {
> -         vec_safe_push (vector, binfo);
> -       }
> -    }
> +    if (BINFO_VIRTUAL_P (binfo))
> +      vec_safe_push (vector, binfo);
>
>    /* Now non-virtuals */
>    for (i = 0; base_binfos->iterate (i, &binfo); i++)
> -    {
> -      if (!BINFO_VIRTUAL_P (binfo))
> -       {
> -         vec_safe_push (vector, binfo);
> -       }
> -    }
> -
> +    if (!BINFO_VIRTUAL_P (binfo))
> +      vec_safe_push (vector, binfo);
>
>    bases_vec = make_tree_vec (vector->length ());
>
>    for (i = 0; i < vector->length (); ++i)
> -    {
> -      TREE_VEC_ELT (bases_vec, i) = BINFO_TYPE ((*vector)[i]);
> -    }
> +    TREE_VEC_ELT (bases_vec, i) = BINFO_TYPE ((*vector)[i]);
> +
>    return bases_vec;
>  }
>
> @@ -3949,9 +3938,7 @@ dfs_calculate_bases_post (tree binfo, vo
>  {
>    vec<tree, va_gc> **data = ((vec<tree, va_gc> **) data_);
>    if (!BINFO_VIRTUAL_P (binfo))
> -    {
> -      vec_safe_push (*data, BINFO_TYPE (binfo));
> -    }
> +    vec_safe_push (*data, BINFO_TYPE (binfo));
>    return NULL_TREE;
>  }
>
> @@ -3959,7 +3946,7 @@ dfs_calculate_bases_post (tree binfo, vo
>  static vec<tree, va_gc> *
>  calculate_bases_helper (tree type)
>  {
> -  vec<tree, va_gc> *vector = make_tree_vector();
> +  vec<tree, va_gc> *vector = make_tree_vector ();
>
>    /* Now add non-virtual base classes in order of construction */
>    if (TYPE_BINFO (type))
> @@ -3969,18 +3956,17 @@ calculate_bases_helper (tree type)
>  }
>
>  tree
> -calculate_bases (tree type)
> +calculate_bases (tree type, tsubst_flags_t complain)
>  {
> -  vec<tree, va_gc> *vector = make_tree_vector();
> +  vec<tree, va_gc> *vector = make_tree_vector ();

While we're touching this line, let's move it down below the early
return so we don't allocate a vec we aren't going to use.  OK with
that change.

>    tree bases_vec = NULL_TREE;
>    unsigned i;
>    vec<tree, va_gc> *vbases;
>    vec<tree, va_gc> *nonvbases;
>    tree binfo;
>
> -  complete_type (type);
> -
> -  if (!NON_UNION_CLASS_TYPE_P (type))
> +  if (!complete_type_or_maybe_complain (type, NULL_TREE, complain)
> +      || !NON_UNION_CLASS_TYPE_P (type))
>      return make_tree_vec (0);
>
>    /* First go through virtual base classes */
> --- gcc/cp/pt.c.jj      2018-04-03 23:39:16.487665280 +0200
> +++ gcc/cp/pt.c 2018-04-04 10:36:03.086316790 +0200
> @@ -11743,15 +11743,18 @@ tsubst_pack_expansion (tree t, tree args
>        int level = 0;
>
>        if (TREE_CODE (parm_pack) == BASES)
> -       {
> -        gcc_assert (parm_pack == pattern);
> -         if (BASES_DIRECT (parm_pack))
> -           return calculate_direct_bases (tsubst_expr (BASES_TYPE (parm_pack),
> -                                                        args, complain, in_decl, false));
> -         else
> -           return calculate_bases (tsubst_expr (BASES_TYPE (parm_pack),
> -                                                 args, complain, in_decl, false));
> -       }
> +       {
> +         gcc_assert (parm_pack == pattern);
> +         if (BASES_DIRECT (parm_pack))
> +           return calculate_direct_bases (tsubst_expr (BASES_TYPE (parm_pack),
> +                                                       args, complain,
> +                                                       in_decl, false),
> +                                          complain);
> +         else
> +           return calculate_bases (tsubst_expr (BASES_TYPE (parm_pack),
> +                                                args, complain, in_decl,
> +                                                false), complain);
> +       }
>        else if (builtin_pack_call_p (parm_pack))
>         {
>           /* ??? Support use in other patterns.  */
> --- gcc/testsuite/g++.dg/ext/bases2.C.jj        2015-12-10 09:48:29.228598346 +0100
> +++ gcc/testsuite/g++.dg/ext/bases2.C   2018-04-04 10:47:57.401384092 +0200
> @@ -5,7 +5,7 @@ template<typename...> struct A {};
>
>  template<typename T> struct B
>  {
> -  typedef A<__bases(T)...> C;
> +  typedef A<__bases(T)...> C;  // { dg-error "incomplete type" }
>  };
>
>  struct X {};
> --- gcc/testsuite/g++.dg/ext/bases3.C.jj        2018-04-04 10:32:46.841305015 +0200
> +++ gcc/testsuite/g++.dg/ext/bases3.C   2018-04-04 10:48:11.646385434 +0200
> @@ -0,0 +1,13 @@
> +// PR c++/85146
> +// { dg-do compile { target c++11 } }
> +
> +template<typename...> struct A {};
> +
> +template<typename T> struct B
> +{
> +  typedef A<__direct_bases(T)...> C;   // { dg-error "incomplete type" }
> +};
> +
> +struct X;
> +
> +B<X> b;
>
>
>         Jakub