amharc accepted this revision.
amharc added a comment.
This revision is now accepted and ready to land.
Looks good to me. Obviously, you should wait for someone more competent than me
to accept it, too.
Repository:
rL LLVM
https://reviews.llvm.org/D47108
amharc requested changes to this revision.
amharc added inline comments.
This revision now requires changes to proceed.
Comment at: clang/test/CodeGenCXX/vtable-available-externally.cpp:445
+// after the Derived construction.
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test187DerivedE =
amharc added inline comments.
Comment at: clang/include/clang/AST/DeclCXX.h:778
+ bool mayBeDynamicClass() const {
+return !isCompleteDefinition() || isDynamicClass();
xbolva00 wrote:
> maybeDynamicClass?
>
>
amharc added a comment.
I think that `MarkVTableUsed` should be called somewhere in Sema (possibly
`ActOnFinishCXXMemberDecls`?) if `ForceEmitVTables` is on. This probably
requires making `ForceEmitVTables` a `LangOption` in addition to it being a
`CodeGenOption`.
This causes the above
amharc requested changes to this revision.
amharc added a comment.
This revision now requires changes to proceed.
This is not sound: sometimes the forcefully emitted vtable is incorrect due to
destructor aliasing. This happens e.g. in the Bullet benchmark from the llvm
test suite. A simplified
