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

[Reid Kleckner]

  LGTM when fixes applied.

  As discussed offline, John is buried under email, so we'll move to post-commit review.


================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:707
@@ +706,3 @@
+    // a unique mangled name.
+    llvm::StringSet<> ObservedMangledNames;
+    for (size_t J = 0, F = VFPtrs.size(); J != F; ++J) {
----------------
Put this in #ifndef NDEBUG.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:673
@@ +672,3 @@
+  llvm::raw_svector_ostream Out(Name);
+  CGM.getCXXABI().getMangleContext().mangleCXXVFTable(
+      RD, VFPtr.PathToMangle, Out);
----------------
I don't know why MangleContext takes streams.  It's not like we ever stream these directly to files.  I may write a patch later to change it to SmallStringImpl &.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:661
@@ +660,3 @@
+  llvm::Value *VTableAddressPoint =
+      CGM.getCXXABI().getAddrOfVTable(VTableClass, Base.getBaseOffset());
+  if (!VTableAddressPoint) {
----------------
No need for "CGM.getCXXABI()" inside MicrosoftCXXABI.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:663
@@ +662,3 @@
+  if (!VTableAddressPoint) {
+    const CXXRecordDecl *BaseDecl = Base.getBase();
+    assert(BaseDecl->getNumVBases() &&
----------------
This will warn in a clang NDEBUG build, probably just fold it into the assert.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:694
@@ +693,3 @@
+
+  llvm::GlobalVariable *&VTable = VFTablesMap[ID];
+
----------------
You can avoid the double DenseMap lookup with .insert(), and you can use llvm:tie() to avoid the ugly std::pair type:

  VFTablesMapTy::iterator I;
  bool Inserted;
  llvm::tie(I, Inserted) = VFTablesMap.insert(0);
  ...


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