Skip to content

Commit dc6ec45

Browse files
committed
[cxx-interop] Assign correct owning module to class template specializations
When importing C++ class template specializations into Swift, we were assigning the owning module to the imported Swift structs inconsistently. For specializations that had a typedef (or a using-decl), we assumed the module that declares the typedef to be the owning module for the specialization. For specializations that do not have a typedef, we assumed the module that declares the class template itself to be the owning module. This changes the behavior to always assume the latter. rdar://158589803
1 parent bf07c4b commit dc6ec45

File tree

7 files changed

+37
-15
lines changed

7 files changed

+37
-15
lines changed

include/swift/AST/ClangModuleLoader.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,8 @@ class ClangModuleLoader : public ModuleLoader {
316316
virtual SwiftLookupTable *
317317
findLookupTable(const clang::Module *clangModule) = 0;
318318

319+
virtual const clang::Module *getClangOwningModule(ClangNode Node) const = 0;
320+
319321
virtual DeclName
320322
importName(const clang::NamedDecl *D,
321323
clang::DeclarationName givenName = clang::DeclarationName()) = 0;

include/swift/ClangImporter/ClangImporter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ class ClangImporter final : public ClangModuleLoader {
487487
bool dumpPrecompiledModule(StringRef modulePath, StringRef outputPath);
488488

489489
bool runPreprocessor(StringRef inputPath, StringRef outputPath);
490-
const clang::Module *getClangOwningModule(ClangNode Node) const;
490+
const clang::Module *getClangOwningModule(ClangNode Node) const override;
491491
bool hasTypedef(const clang::Decl *typeDecl) const;
492492

493493
void verifyAllModules() override;

lib/AST/Decl.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -932,16 +932,18 @@ static ModuleDecl *getModuleContextForNameLookupForCxxDecl(const Decl *decl) {
932932
if (!ctx.LangOpts.EnableCXXInterop)
933933
return nullptr;
934934

935+
auto clangImporter = ctx.getClangModuleLoader();
936+
ASSERT(clangImporter && "ClangImporter required");
937+
935938
// When we clone members for base classes the cloned members have no
936939
// corresponding Clang nodes. Look up the original imported declaration to
937940
// figure out what Clang module does the cloned member originate from.
938941
bool isClonedMember = false;
939942
if (auto VD = dyn_cast<ValueDecl>(decl))
940-
if (auto loader = ctx.getClangModuleLoader())
941-
if (auto original = loader->getOriginalForClonedMember(VD)) {
942-
isClonedMember = true;
943-
decl = original;
944-
}
943+
if (auto original = clangImporter->getOriginalForClonedMember(VD)) {
944+
isClonedMember = true;
945+
decl = original;
946+
}
945947

946948
if (!decl->hasClangNode())
947949
return nullptr;
@@ -956,15 +958,15 @@ static ModuleDecl *getModuleContextForNameLookupForCxxDecl(const Decl *decl) {
956958
return nullptr;
957959
}
958960

959-
auto clangModule = decl->getClangDecl()->getOwningModule();
961+
auto clangModule = clangImporter->getClangOwningModule(decl->getClangDecl());
960962
if (!clangModule)
961963
return nullptr;
962964

963965
// Swift groups all submodules of a Clang module together, and imports them as
964966
// a single top-level module.
965967
clangModule = clangModule->getTopLevelModule();
966968

967-
return ctx.getClangModuleLoader()->getWrapperForModule(clangModule);
969+
return clangImporter->getWrapperForModule(clangModule);
968970
}
969971

970972
ModuleDecl *Decl::getModuleContextForNameLookup() const {

lib/ClangImporter/ClangImporter.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3199,13 +3199,13 @@ getClangOwningModule(ClangNode Node, const clang::ASTContext &ClangCtx) {
31993199
originalDecl = pattern;
32003200
}
32013201
}
3202-
if (!originalDecl->hasOwningModule()) {
3203-
if (auto cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(D)) {
3204-
if (auto pattern = cxxRecordDecl->getTemplateInstantiationPattern()) {
3205-
// Class template instantiations sometimes don't have an owning Clang
3206-
// module, if the instantiation is not typedef-ed.
3207-
originalDecl = pattern;
3208-
}
3202+
if (auto classTemplateSpecDecl =
3203+
dyn_cast<clang::ClassTemplateSpecializationDecl>(D)) {
3204+
if (auto pattern =
3205+
classTemplateSpecDecl->getTemplateInstantiationPattern()) {
3206+
// Class template instantiations sometimes don't have an owning Clang
3207+
// module, if the instantiation is not typedef-ed.
3208+
originalDecl = pattern;
32093209
}
32103210
}
32113211

test/Interop/Cxx/stdlib/Inputs/module.modulemap

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ module StdVector {
1616
export *
1717
}
1818

19+
module StdVectorNoCPlusPlusRequirement {
20+
header "std-vector-no-cplusplus-requirement.h"
21+
// NO requires cplusplus
22+
export *
23+
}
24+
1925
module StdSpan {
2026
header "std-span.h"
2127
requires cplusplus
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#include <vector>
2+
3+
using VectorOfFloat = std::vector<float>;

test/Interop/Cxx/stdlib/use-std-vector-typechecker.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// XFAIL: OS=linux-androideabi
33

44
import StdVector
5+
import StdVectorNoCPlusPlusRequirement
56
import CxxStdlib
67

78
func takeCopyable<T: Copyable>(_ x: T) {}
@@ -20,3 +21,11 @@ let vecPointer = VectorOfPointer()
2021
takeCopyable(vecPointer)
2122
takeCxxVector(vecPointer)
2223
// CHECK-NOT: error
24+
25+
// Make sure that a specialization of std::vector that is declared in a Clang
26+
// module which does not declare 'requires cplusplus' is still conformed to
27+
// CxxVector.
28+
let vecFloat = VectorOfFloat()
29+
takeCopyable(vecFloat)
30+
takeCxxVector(vecFloat)
31+
// CHECK-NOT: error

0 commit comments

Comments
 (0)