[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