[PATCH] Support MS Non-Virtual Base Placement Rules in CGRecordLayoutBuilder

[David Majnemer]

================
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