[PATCH] D12385: Generating Assumption loads fix

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/CodeGen/CGCXXABI.h:221
@@ -220,4 +220,3 @@
 
-  virtual bool canEmitAvailableExternallyVTable(
-      const CXXRecordDecl *RD) const = 0;
+  virtual bool canEmitUnusedVTable(const CXXRecordDecl *RD) const = 0;
 
----------------
This would benefit from a documentation comment:

> Determine whether it's possible to emit a vtable for \p RD, even though we do not know that the vtable has been marked as used by semantic analysis.

I'm also not entirely happy with this name (even though I think I suggested it). `canSpeculativelyEmitVTable` might be better.

================
Comment at: test/CodeGenCXX/vtable-assume-load.cpp:174
@@ -171,2 +173,3 @@
 
 } // test5
+
----------------
Comment doesn't match namespace name `testMS`.

================
Comment at: test/CodeGenCXX/vtable-assume-load.cpp:268-269
@@ +267,4 @@
+
+// Vtable is generated in this TU, but C has inline virtual functions which
+// prohibits as from generating assumption loads.
+// CHECK8-LABEL: define void @_ZN5test81cEv()
----------------
Maybe mark this one with FIXME:


http://reviews.llvm.org/D12385