================
Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:149
@@ +148,3 @@
+ /// like MSVC.
+ bool MSLayoutNonVirtualBases(const CXXRecordDecl *RD,
+ const ASTRecordLayout &Layout);
----------------
Method is called `MSLayoutNonVirtualBases` but comment refers to it as
`LayoutNonVirtualBases`
Also, the method name implies that it only performs layout for non-virtual
bases but the comment explicitly states that it only performs layout for
virtual bases.
================
Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:832
@@ -759,1 +831,3 @@
+ return false;
+ } else if (!LayoutNonVirtualBases(RD, Layout))
return false;
----------------
It makes more sense, at least to me, that `LayoutNonVirtualBases` dispatch to
`MSLayoutNonVirtualBases` and `ItaniumLayoutNonVirtualBases` instead of having
`LayoutNonVirtualBases` implicitly be the Itanium implementation.
================
Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:735
@@ +734,3 @@
+
+ // Add a vfptr if the layout says to do so.
+ if (Layout.hasOwnVFPtr()) {
----------------
Can we have a series of nv-bases followed by **our** `vfptr` followed by a
different series of nv-bases?
Can we have a `vfptr` that proceeds a bunch of nv-bases without a leading
`vfptr`?
If the answer to both of these are no, could we sink the code that appends the
`vfptr` field right above the code that handles the `vbptr`?
http://llvm-reviews.chandlerc.com/D2524
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
