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

Reply via email to