Skip to content

Commit f4e74f7

Browse files
authored
When qualifying Clang types with a module, make sure we choose a visible module (#32465)
Clang types need special treatment because multiple Clang modules can contain the same type declarations from a textually included header, but not all of these modules may be visible. This fixes https://bugs.swift.org/browse/SR-13032 The newly added test breaks without this fix.
1 parent 3a27c11 commit f4e74f7

File tree

6 files changed

+109
-8
lines changed

6 files changed

+109
-8
lines changed

include/swift/AST/PrintOptions.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,8 @@ struct PrintOptions {
550550
/// attributes.
551551
///
552552
/// \see swift::emitSwiftInterface
553-
static PrintOptions printSwiftInterfaceFile(bool preferTypeRepr,
553+
static PrintOptions printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
554+
bool preferTypeRepr,
554555
bool printFullConvention,
555556
bool printSPIs);
556557

lib/AST/ASTPrinter.cpp

Lines changed: 89 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,16 @@ static bool contributesToParentTypeStorage(const AbstractStorageDecl *ASD) {
107107
return !ND->isResilient() && ASD->hasStorage() && !ASD->isStatic();
108108
}
109109

110-
PrintOptions PrintOptions::printSwiftInterfaceFile(bool preferTypeRepr,
110+
PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
111+
bool preferTypeRepr,
111112
bool printFullConvention,
112113
bool printSPIs) {
113114
PrintOptions result;
114115
result.IsForSwiftInterface = true;
115116
result.PrintLongAttrsOnSeparateLines = true;
116117
result.TypeDefinitions = true;
117118
result.PrintIfConfig = false;
119+
result.CurrentModule = ModuleToPrint;
118120
result.FullyQualifiedTypes = true;
119121
result.UseExportedModuleNames = true;
120122
result.AllowNullTypes = false;
@@ -3648,6 +3650,8 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
36483650

36493651
ASTPrinter &Printer;
36503652
const PrintOptions &Options;
3653+
Optional<llvm::DenseMap<const clang::Module *, ModuleDecl *>>
3654+
VisibleClangModules;
36513655

36523656
void printGenericArgs(ArrayRef<Type> Args) {
36533657
if (Args.empty())
@@ -3678,7 +3682,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
36783682
}
36793683
}
36803684

3681-
/// Determinee whether the given type has a simple representation
3685+
/// Determine whether the given type has a simple representation
36823686
/// under the current print options.
36833687
bool isSimpleUnderPrintOptions(Type T) {
36843688
if (auto typealias = dyn_cast<TypeAliasType>(T.getPointer())) {
@@ -3700,19 +3704,100 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
37003704
return T->hasSimpleTypeRepr();
37013705
}
37023706

3707+
/// Computes the map that is cached by `getVisibleClangModules()`.
3708+
/// Do not call directly.
3709+
llvm::DenseMap<const clang::Module *, ModuleDecl *>
3710+
computeVisibleClangModules() {
3711+
assert(Options.CurrentModule &&
3712+
"CurrentModule needs to be set to determine imported Clang modules");
3713+
3714+
llvm::DenseMap<const clang::Module *, ModuleDecl *> Result;
3715+
3716+
// For the current module, consider both private and public imports.
3717+
ModuleDecl::ImportFilter Filter = ModuleDecl::ImportFilterKind::Exported;
3718+
Filter |= ModuleDecl::ImportFilterKind::Default;
3719+
Filter |= ModuleDecl::ImportFilterKind::SPIAccessControl;
3720+
SmallVector<ImportedModule, 4> Imports;
3721+
Options.CurrentModule->getImportedModules(Imports, Filter);
3722+
3723+
SmallVector<ModuleDecl *, 4> ModulesToProcess;
3724+
for (const auto &Import : Imports) {
3725+
ModulesToProcess.push_back(Import.importedModule);
3726+
}
3727+
3728+
SmallPtrSet<ModuleDecl *, 4> Processed;
3729+
while (!ModulesToProcess.empty()) {
3730+
ModuleDecl *Mod = ModulesToProcess.back();
3731+
ModulesToProcess.pop_back();
3732+
3733+
if (!Processed.insert(Mod).second)
3734+
continue;
3735+
3736+
if (const clang::Module *ClangModule = Mod->findUnderlyingClangModule())
3737+
Result[ClangModule] = Mod;
3738+
3739+
// For transitive imports, consider only public imports.
3740+
Imports.clear();
3741+
Mod->getImportedModules(Imports, ModuleDecl::ImportFilterKind::Exported);
3742+
for (const auto &Import : Imports) {
3743+
ModulesToProcess.push_back(Import.importedModule);
3744+
}
3745+
}
3746+
3747+
return Result;
3748+
}
3749+
3750+
/// Returns all Clang modules that are visible from `Options.CurrentModule`.
3751+
/// This includes any modules that are imported transitively through public
3752+
/// (`@_exported`) imports.
3753+
///
3754+
/// The returned map associates each visible Clang module with the
3755+
/// corresponding Swift module.
3756+
const llvm::DenseMap<const clang::Module *, ModuleDecl *> &
3757+
getVisibleClangModules() {
3758+
if (!VisibleClangModules) {
3759+
VisibleClangModules = computeVisibleClangModules();
3760+
}
3761+
return *VisibleClangModules;
3762+
}
3763+
37033764
template <typename T>
37043765
void printModuleContext(T *Ty) {
37053766
FileUnit *File = cast<FileUnit>(Ty->getDecl()->getModuleScopeContext());
37063767
ModuleDecl *Mod = File->getParentModule();
3768+
StringRef ExportedModuleName = File->getExportedModuleName();
3769+
3770+
// Clang declarations need special treatment: Multiple Clang modules can
3771+
// contain the same declarations from a textually included header, but not
3772+
// all of these modules may be visible. We therefore need to make sure we
3773+
// choose a module that is visible from the current module. This is possible
3774+
// only if we know what the current module is.
3775+
const clang::Decl *ClangDecl = Ty->getDecl()->getClangDecl();
3776+
if (ClangDecl && Options.CurrentModule) {
3777+
for (auto *Redecl : ClangDecl->redecls()) {
3778+
clang::Module *ClangModule =
3779+
Redecl->getOwningModule()->getTopLevelModule();
3780+
if (!ClangModule)
3781+
continue;
3782+
3783+
if (ModuleDecl *VisibleModule =
3784+
getVisibleClangModules().lookup(ClangModule)) {
3785+
Mod = VisibleModule;
3786+
ExportedModuleName = ClangModule->ExportAsModule;
3787+
break;
3788+
}
3789+
}
3790+
}
37073791

37083792
if (Options.MapCrossImportOverlaysToDeclaringModule) {
37093793
if (ModuleDecl *Declaring = Mod->getDeclaringModuleIfCrossImportOverlay())
37103794
Mod = Declaring;
37113795
}
37123796

37133797
Identifier Name = Mod->getName();
3714-
if (Options.UseExportedModuleNames)
3715-
Name = Mod->getASTContext().getIdentifier(File->getExportedModuleName());
3798+
if (Options.UseExportedModuleNames && !ExportedModuleName.empty()) {
3799+
Name = Mod->getASTContext().getIdentifier(ExportedModuleName);
3800+
}
37163801

37173802
Printer.printModuleRef(Mod, Name);
37183803
Printer << ".";

lib/Frontend/ModuleInterfaceSupport.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ bool swift::emitSwiftInterface(raw_ostream &out,
517517
printImports(out, Opts, M);
518518

519519
const PrintOptions printOptions = PrintOptions::printSwiftInterfaceFile(
520-
Opts.PreserveTypesAsWritten, Opts.PrintFullConvention, Opts.PrintSPIs);
520+
M, Opts.PreserveTypesAsWritten, Opts.PrintFullConvention, Opts.PrintSPIs);
521521
InheritedProtocolCollector::PerTypeMap inheritedProtocolMap;
522522

523523
SmallVector<Decl *, 16> topLevelDecls;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
@_spi(dummy) import HelperModule
2+
3+
public func funcTakingForeignStruct(_ param: ForeignStruct) {}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// This tests verifies that SPI visible modules are understood by the machinery
2+
// that produces .swiftinterface files.
3+
//
4+
// See the documentation for the print-qualified-clang-types.swift test next to
5+
// this one for more context.
6+
7+
// RUN: %empty-directory(%t)
8+
// RUN: mkdir %t/helper_module %t/spi_main_module
9+
// RUN: %target-swift-frontend -enable-library-evolution -swift-version 5 -emit-module -o %t/helper_module/HelperModule.swiftmodule %S/Inputs/HelperModule.swift -I %S/Inputs
10+
// RUN: %target-swift-frontend -enable-library-evolution -swift-version 5 -emit-module -o %t/spi_main_module/SpiMainModule.swiftmodule -emit-module-interface-path %t/spi_main_module/SpiMainModule.swiftinterface -I %t/helper_module %S/Inputs/SpiMainModule.swift -I %S/Inputs
11+
// RUN: %FileCheck --input-file=%t/spi_main_module/SpiMainModule.swiftinterface %s
12+
// RUN: %target-swift-frontend -typecheck -swift-version 5 %t/spi_main_module/SpiMainModule.swiftinterface -I %t/helper_module -I %S/Inputs
13+
14+
// CHECK: public func funcTakingForeignStruct(_ param: ForeignB.ForeignStruct)

test/Interop/C/modules/print-qualified-clang-types/print-qualified-clang-types.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,3 @@
3434
// RUN: %target-swift-frontend -typecheck -swift-version 5 %t/main_module/MainModule.swiftinterface -I %t/helper_module -I %S/Inputs
3535

3636
// CHECK: public func funcTakingForeignStruct(_ param: ForeignB.ForeignStruct)
37-
38-
// REQUIRES: SR-13032

0 commit comments

Comments
 (0)