Skip to content

Commit

Permalink
[C++20] [Modules] Correct the linkage for template instantiations in
Browse files Browse the repository at this point in the history
named modules

Close #97313

In the previous patch (#75912),
I made an oversight that I ignored the templates in named module when
calculating the linkage for the vtables. In this patch, I tried to
correct the behavior by merging the logics to calculate the linkage with
key functions with named modules.
  • Loading branch information
ChuanqiXu9 committed Jul 2, 2024
1 parent c72cb27 commit 18f3bcb
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 19 deletions.
43 changes: 24 additions & 19 deletions clang/lib/CodeGen/CGVTables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1079,33 +1079,38 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {
if (!RD->isExternallyVisible())
return llvm::GlobalVariable::InternalLinkage;

// V-tables for non-template classes with an owning module are always
// uniquely emitted in that module.
if (RD->isInNamedModule())
return llvm::GlobalVariable::ExternalLinkage;

// We're at the end of the translation unit, so the current key
// function is fully correct.
const CXXMethodDecl *keyFunction = Context.getCurrentKeyFunction(RD);
if (keyFunction && !RD->hasAttr<DLLImportAttr>()) {
bool IsInNamedModule = RD->isInNamedModule();
// If the CXXRecordDecl are not in a module unit, we need to get
// its key function. We're at the end of the translation unit, so the current
// key function is fully correct.
const CXXMethodDecl *keyFunction =
IsInNamedModule ? nullptr : Context.getCurrentKeyFunction(RD);
if (IsInNamedModule || (keyFunction && !RD->hasAttr<DLLImportAttr>())) {
// If this class has a key function, use that to determine the
// linkage of the vtable.
const FunctionDecl *def = nullptr;
if (keyFunction->hasBody(def))
if (keyFunction && keyFunction->hasBody(def))
keyFunction = cast<CXXMethodDecl>(def);

switch (keyFunction->getTemplateSpecializationKind()) {
case TSK_Undeclared:
case TSK_ExplicitSpecialization:
bool IsExternalDefinition =
IsInNamedModule ? RD->shouldEmitInExternalSource() : !def;

TemplateSpecializationKind Kind =
IsInNamedModule ? RD->getTemplateSpecializationKind()
: keyFunction->getTemplateSpecializationKind();

switch (Kind) {
case TSK_Undeclared:
case TSK_ExplicitSpecialization:
assert(
(def || CodeGenOpts.OptimizationLevel > 0 ||
(IsInNamedModule || def || CodeGenOpts.OptimizationLevel > 0 ||
CodeGenOpts.getDebugInfo() != llvm::codegenoptions::NoDebugInfo) &&
"Shouldn't query vtable linkage without key function, "
"optimizations, or debug info");
if (!def && CodeGenOpts.OptimizationLevel > 0)
"Shouldn't query vtable linkage without the class in module units, "
"key function, optimizations, or debug info");
if (IsExternalDefinition && CodeGenOpts.OptimizationLevel > 0)
return llvm::GlobalVariable::AvailableExternallyLinkage;

if (keyFunction->isInlined())
if (keyFunction && keyFunction->isInlined())
return !Context.getLangOpts().AppleKext
? llvm::GlobalVariable::LinkOnceODRLinkage
: llvm::Function::InternalLinkage;
Expand All @@ -1124,7 +1129,7 @@ CodeGenModule::getVTableLinkage(const CXXRecordDecl *RD) {

case TSK_ExplicitInstantiationDeclaration:
llvm_unreachable("Should not have been asked to emit this");
}
}
}

// -fapple-kext mode does not support weak linkage, so we must use
Expand Down
118 changes: 118 additions & 0 deletions clang/test/Modules/pr97313.cppm
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// REQUIRES: !system-windows
//
// RUN: rm -rf %t
// RUN: mkdir -p %t
// RUN: split-file %s %t
//
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Base.cppm \
// RUN: -emit-module-interface -o %t/Base.pcm
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Sub.cppm \
// RUN: -emit-module-interface -o %t/Sub.pcm -fprebuilt-module-path=%t
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Sub.pcm \
// RUN: -emit-llvm -o %t/Sub.pcm -o - -fprebuilt-module-path=%t | \
// RUN: FileCheck %t/Sub.cppm
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/main.cpp \
// RUN: -emit-llvm -fprebuilt-module-path=%t -o - | FileCheck %t/main.cpp
//
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Mod.cppm \
// RUN: -emit-module-interface -o %t/Mod.pcm
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Mod.pcm \
// RUN: -emit-llvm -o - | FileCheck %t/Mod.cppm
// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/Use.cpp \
// RUN: -emit-llvm -fprebuilt-module-path=%t -o - | \
// RUN: FileCheck %t/Use.cpp

//--- Base.cppm
export module Base;

export template <class>
class Base
{
public:
constexpr Base();
constexpr virtual ~Base();
};

template <class X>
constexpr Base<X>::Base() = default;

template <class X>
constexpr Base<X>::~Base() = default;

//--- Sub.cppm
export module Sub;
export import Base;

export class Sub : public Base<int>
{
};

// CHECK: @_ZTIW4Base4BaseIiE = {{.*}}linkonce_odr

//--- main.cpp
import Sub;

int main()
{
Base<int> *b = new Sub();
delete b;
}

// CHECK: @_ZTIW4Base4BaseIiE = {{.*}}linkonce_odr

//--- Mod.cppm
export module Mod;

export class NonTemplate {
public:
virtual ~NonTemplate();
};

// CHECK: @_ZTIW3Mod11NonTemplate = {{.*}}constant

export template <class C>
class Template {
public:
virtual ~Template();
};

export template<>
class Template<char> {
public:
virtual ~Template();
};

// CHECK: @_ZTIW3Mod8TemplateIcE = {{.*}}constant

export template class Template<unsigned>;

// CHECK: @_ZTIW3Mod8TemplateIjE = {{.*}}weak_odr

export extern template class Template<double>;

auto v = new Template<signed int>();

// CHECK: @_ZTIW3Mod8TemplateIiE = {{.*}}linkonce_odr

//--- Use.cpp
import Mod;

auto v1 = new NonTemplate();
auto v2 = new Template<char>();
auto v3 = new Template<unsigned>();
auto v4 = new Template<double>();
auto v5 = new Template<signed int>();
auto v6 = new Template<NonTemplate>();

// CHECK: @_ZTVW3Mod11NonTemplate = {{.*}}external
// CHECK: @_ZTVW3Mod8TemplateIcE = {{.*}}external
// CHECK: @_ZTVW3Mod8TemplateIjE = {{.*}}weak_odr
// CHECK: @_ZTSW3Mod8TemplateIjE = {{.*}}weak_odr
// CHECK: @_ZTIW3Mod8TemplateIjE = {{.*}}weak_odr
// CHECK: @_ZTVW3Mod8TemplateIdE = {{.*}}external
// CHECK: @_ZTVW3Mod8TemplateIiE = {{.*}}linkonce_odr
// CHECK: @_ZTSW3Mod8TemplateIiE = {{.*}}linkonce_odr
// CHECK: @_ZTIW3Mod8TemplateIiE = {{.*}}linkonce_odr
// CHECK: @_ZTVW3Mod8TemplateIS_11NonTemplateE = {{.*}}linkonce_odr
// CHECK: @_ZTSW3Mod8TemplateIS_11NonTemplateE = {{.*}}linkonce_odr
// CHECK: @_ZTIW3Mod8TemplateIS_11NonTemplateE = {{.*}}linkonce_odr

0 comments on commit 18f3bcb

Please sign in to comment.