[PATCH] Abstract out the emission of vtables, add basic support for vtable emission when using -cxx-abi microsoft

[Timur Iskhodzhanov]

================
Comment at: lib/AST/VTableBuilder.cpp:2322
@@ -2354,3 +2321,3 @@
                           Builder.getAddressPoints(),
-                          Builder.isMicrosoftABI());
+                          false);
 }
----------------
Reid Kleckner wrote:
> /*IsMicrosoftABI=*/
Done

================
Comment at: lib/CodeGen/CGExprConstant.cpp:56
@@ -55,4 +55,3 @@
 
-  void AppendVTablePointer(BaseSubobject Base, llvm::Constant *VTable,
-                           const CXXRecordDecl *VTableClass);
+  void AppendVTablePointer(BaseSubobject Base, const CXXRecordDecl *VTableClass);
 
----------------
Reid Kleckner wrote:
> This method is not defined, so you can drop it.
Good catch - I forgot to remove the declaration.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:325
@@ +324,3 @@
+
+  /// EmittedVFTables - Holds the set of the record decls we've deferred vtable
+  /// emission for.
----------------
Reid Kleckner wrote:
> "EmittedVFTables" is wrong.  Maybe just \brief?
Done; changed the VBTablesMap comment too for consistency.

================
Comment at: include/clang/AST/Mangle.h:116
@@ -111,1 +115,3 @@
+                                const CXXRecordDecl *LastVBase,
+                                bool TheOnlyVFTable, raw_ostream &) = 0;
   virtual void mangleCXXVTT(const CXXRecordDecl *RD,
----------------
Reid Kleckner wrote:
> Ultimately, for vbtables, I needed to walk the entire inheritance graph of the class to figure out how things were mangled.  As a result, I ended up feeding the mangler a carefully constructed BasePath and dropping all the complex logic from the mangler.  Long term, we'll have to do the same here, so I'd drop TheOnlyVFTable and LastVBase and adjust the BasePath before feeding it to the mangler.
OK, done

================
Comment at: lib/AST/MicrosoftMangle.cpp:1873
@@ +1872,3 @@
+  Mangler.getStream() << "6B"; // '6' for vftable, 'B' for const.
+  if (!TheOnlyVFTable) {
+    // FIXME: In some rare cases this code produces a bit incorrect mangling.
----------------
Reid Kleckner wrote:
> Yep, I'd move this all out to the caller.
Agreed, done

================
Comment at: include/clang/AST/Mangle.h:109
@@ -108,1 +108,3 @@
                                         raw_ostream &) = 0;
+  // FIXME: Some of these objects only exist in select ABIs. We should probably
+  // only declare them in ABI-specific manglers?
----------------
Reid Kleckner wrote:
> A MicrosoftMangle.h which only exposes MicrosoftMangleContext and is only included by MicrosoftCXXABI.cpp (ditto for Itanium)?  Sounds like a good separate patch.
> 
Would you mind if I do this after landing this?
Also, it'd be a good time to rename VTable{Context,Builder} -> ItaniumVTable{Context,Builder} then.


http://llvm-reviews.chandlerc.com/D1532