[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)
John McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 5 11:20:26 PST 2024
================
@@ -1046,6 +1046,15 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
if (!RD->isExternallyVisible())
return llvm::GlobalVariable::InternalLinkage;
+ // Previously we'll decide the linkage of the vtable by the linkage
+ // of the key function. But within modules, the concept of key functions
+ // becomes meaningless. So the linkage of the vtable should always be
+ // external if the class is externally visible.
+ //
+ // TODO: How about the case of AppleKext, DLLExportAttr and DLLImportAttr.
----------------
rjmccall wrote:
Comments should be timeless: they exist to explain the current code, not the patch you're making. So don't say stuff like "previously". You can just "V-tables for non-template classes with an owning module are always uniquely emitted in that module."
It's correct for this to apply even for the Apple kext, dllexport, and dllimport cases. But I think you do actually need to handle the template case here; we can't emit v-tables for template instantiations with strong linkage.
I think this function could probably be *significantly* simplified to eliminate a lot of the redundancy; it's essentially:
- Use internal linkage if the class isn't externally visible.
- Decide whether the v-table has vague linkage.
- If not, it gets either external or available-externally linkage based on a few different criteria.
- If so, there's a bunch of platform/ABI-specific decisions that affect the exact LLVM linkage we pick.
https://github.com/llvm/llvm-project/pull/75912
More information about the cfe-commits
mailing list