Intrinsics for N2965: Type traits and base classes

Jason Merrill jason@redhat.com
Mon Sep 26 18:40:00 GMT 2011


On 09/26/2011 11:50 AM, Mike Spertus wrote:
> This patch consists intrinsics to properly create the bases and direct_bases of a class

Looks pretty good.  Some comments:

>   #define GCC_TREE_H
> -
>   #include "hashtab.h"

I don't see any reason to remove this blank line.

> +      if (TREE_CODE (parm_pack) == BASES)
> +        {
> +          return calculate_bases(tsubst_expr(BASES_TYPE(parm_pack),
> +                                 args, complain, in_decl, false));
> +        }
> +      if (TREE_CODE (parm_pack) == DIRECT_BASES)
> +        {
> +          return calculate_direct_bases(tsubst_expr(DIRECT_BASES_TYPE(parm_pack),
> +                                        args, complain, in_decl, false));
> +        }

Need spaces before '(' in C code.  And we usually don't put braces 
around a single statement.

> +/* Implement the __direct_bases keyword: Return the direct base classes
> +   of type */
> +tree

Add a blank line between comment and return type.

> +  if (!NON_UNION_CLASS_TYPE_P (type))
> +    {
> +      return bases_vec;
> +    }

More braces.

> +  /* Virtual bases are initialized first */
> +  vector = VEC_copy (tree, gc, CLASSTYPE_VBASECLASSES (type));

This will get you indirect virtual bases as well as direct.

> +  base_binfos = BINFO_BASE_BINFOS(TYPE_BINFO (complete_type (TYPE_MAIN_VARIANT (type))));

Another missing space.  And you don't need either complete_type or 
TYPE_MAIN_VARIANT here; all variants get completed together by the 
complete_type earlier in the function, and TYPE_BINFO is the same for 
all variants.

> +  /* First go through virtual base classes */
> +  for (vbases = CLASSTYPE_VBASECLASSES (type), i = 0;
> +       VEC_iterate (tree, vbases, i, binfo); i++)
> +    {
> +      VEC_safe_splice (tree, gc, vector, calculate_bases_helper (BINFO_TYPE (binfo)));
> +    }

Looks like you'll get duplicates if a virtual base itself has virtual bases.

Let's release_tree_vector the returned vector after we've spliced it on.

> +  dfs_walk_all (TYPE_BINFO (complete_type (TYPE_MAIN_VARIANT (type))),

Again, TYPE_BINFO (type) should be enough.

> +DEFTREECODE (BASES, "bases", tcc_type, 0)
> +DEFTREECODE (DIRECT_BASES, "direct_bases", tcc_type, 0)

Instead of two tree codes, let's use one and a flag.  Also combine 
finish_bases and finish_direct_bases.

Jason



More information about the Gcc-patches mailing list