rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:399-400
@@ +398,4 @@
+ return true;
+ }
+ else if (VtableComponent.isUsedFunctionPointerKind()) {
+ const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
----------------
No newline before `else`.
================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:1632
@@ -1611,3 +1631,3 @@
bool ItaniumCXXABI::canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const {
// We don't emit available_externally vtables if we are in -fapple-kext mode
----------------
This function is now serving two purposes:
1) Can we generate a new reference to the vtable that was not implied by the
source code and checked by `Sema`, and
2) Can we generate a speculative definition of the vtable, despite it (perhaps)
not being used.
I'm OK with the current approach as a conservative fix for the issue we're
seeing with visibility, but we should aim to separate out these two questions
eventually. For the former, we don't need to check the vtable contents, just
the visibility of the vtable symbol itself.
================
Comment at: test/CodeGenCXX/vtable-assume-load.cpp:186
@@ -185,2 +185,3 @@
};
-// Because A's vtable is external, it's safe to generate assumption loads.
+// FIXME: Because A's vtable is external, and all functions aint hidden,
+// it's safe to generate assumption loads.
----------------
"and all functions aint hidden," -> "and no virtual functions are hidden,"
http://reviews.llvm.org/D12865
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
