From 28eb9e042c7a43c1e2546e6a785aceaf9e60d89c Mon Sep 17 00:00:00 2001 From: Martin Boehme Date: Wed, 17 Jun 2020 11:35:30 +0200 Subject: [PATCH 01/11] Repro for an invalid .swiftinterface generated when textually including a C struct in different modules. The newly added test fails. --- test/Interop/C/modules/Inputs/MainModule.swift | 3 +++ test/Interop/C/modules/Inputs/SubModule.swift | 2 ++ test/Interop/C/modules/Inputs/foreign-a.h | 1 + test/Interop/C/modules/Inputs/foreign-b.h | 1 + test/Interop/C/modules/Inputs/module.modulemap | 7 +++++++ test/Interop/C/modules/Inputs/textual-header.h | 1 + test/Interop/C/modules/repro.swift | 5 +++++ 7 files changed, 20 insertions(+) create mode 100644 test/Interop/C/modules/Inputs/MainModule.swift create mode 100644 test/Interop/C/modules/Inputs/SubModule.swift create mode 100644 test/Interop/C/modules/Inputs/foreign-a.h create mode 100644 test/Interop/C/modules/Inputs/foreign-b.h create mode 100644 test/Interop/C/modules/Inputs/module.modulemap create mode 100644 test/Interop/C/modules/Inputs/textual-header.h create mode 100644 test/Interop/C/modules/repro.swift diff --git a/test/Interop/C/modules/Inputs/MainModule.swift b/test/Interop/C/modules/Inputs/MainModule.swift new file mode 100644 index 0000000000000..e4f4d9aef3691 --- /dev/null +++ b/test/Interop/C/modules/Inputs/MainModule.swift @@ -0,0 +1,3 @@ +import SubModule + +public func funcTakingForeignStruct(_ param: ForeignStruct) {} diff --git a/test/Interop/C/modules/Inputs/SubModule.swift b/test/Interop/C/modules/Inputs/SubModule.swift new file mode 100644 index 0000000000000..38cd0e7b41e91 --- /dev/null +++ b/test/Interop/C/modules/Inputs/SubModule.swift @@ -0,0 +1,2 @@ +import ForeignA +@_exported import ForeignB diff --git a/test/Interop/C/modules/Inputs/foreign-a.h b/test/Interop/C/modules/Inputs/foreign-a.h new file mode 100644 index 0000000000000..ca297206959cc --- /dev/null +++ b/test/Interop/C/modules/Inputs/foreign-a.h @@ -0,0 +1 @@ +#include "textual-header.h" diff --git a/test/Interop/C/modules/Inputs/foreign-b.h b/test/Interop/C/modules/Inputs/foreign-b.h new file mode 100644 index 0000000000000..ca297206959cc --- /dev/null +++ b/test/Interop/C/modules/Inputs/foreign-b.h @@ -0,0 +1 @@ +#include "textual-header.h" diff --git a/test/Interop/C/modules/Inputs/module.modulemap b/test/Interop/C/modules/Inputs/module.modulemap new file mode 100644 index 0000000000000..6e3bd2f43ad45 --- /dev/null +++ b/test/Interop/C/modules/Inputs/module.modulemap @@ -0,0 +1,7 @@ +module ForeignA { + header "foreign-a.h" +} + +module ForeignB { + header "foreign-b.h" +} diff --git a/test/Interop/C/modules/Inputs/textual-header.h b/test/Interop/C/modules/Inputs/textual-header.h new file mode 100644 index 0000000000000..e99c9349c57a9 --- /dev/null +++ b/test/Interop/C/modules/Inputs/textual-header.h @@ -0,0 +1 @@ +struct ForeignStruct {}; diff --git a/test/Interop/C/modules/repro.swift b/test/Interop/C/modules/repro.swift new file mode 100644 index 0000000000000..a556d9ad8f714 --- /dev/null +++ b/test/Interop/C/modules/repro.swift @@ -0,0 +1,5 @@ +// RUN: %empty-directory(%t) +// RUN: mkdir %t/sub_module %t/main_module +// RUN: %target-swift-frontend -enable-library-evolution -swift-version 5 -emit-module -o %t/sub_module/SubModule.swiftmodule %S/Inputs/SubModule.swift -I %S/Inputs +// RUN: %target-swift-frontend -enable-library-evolution -swift-version 5 -emit-module -o %t/main_module/MainModule.swiftmodule -emit-module-interface-path %t/main_module/MainModule.swiftinterface -I %t/sub_module %S/Inputs/MainModule.swift -I %S/Inputs +// RUN: %target-swift-frontend -typecheck -swift-version 5 %t/main_module/MainModule.swiftinterface -I %t/sub_module -I %S/Inputs From 39bb5a131daf2203abf82b641e5ee02ff50bd5b0 Mon Sep 17 00:00:00 2001 From: Martin Boehme Date: Thu, 18 Jun 2020 15:15:00 +0200 Subject: [PATCH 02/11] When qualifying Clang types with a module, make sure we choose a visible module. 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 --- include/swift/AST/PrintOptions.h | 3 +- lib/AST/ASTPrinter.cpp | 71 +++++++++++++++++++++++-- lib/Frontend/ModuleInterfaceSupport.cpp | 2 +- 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/include/swift/AST/PrintOptions.h b/include/swift/AST/PrintOptions.h index ebdf2db947a5e..22537e677ffcc 100644 --- a/include/swift/AST/PrintOptions.h +++ b/include/swift/AST/PrintOptions.h @@ -550,7 +550,8 @@ struct PrintOptions { /// attributes. /// /// \see swift::emitSwiftInterface - static PrintOptions printSwiftInterfaceFile(bool preferTypeRepr, + static PrintOptions printSwiftInterfaceFile(ModuleDecl *M, + bool preferTypeRepr, bool printFullConvention, bool printSPIs); diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index d1718a376c44b..95f5ec0dbd720 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -107,7 +107,8 @@ static bool contributesToParentTypeStorage(const AbstractStorageDecl *ASD) { return !ND->isResilient() && ASD->hasStorage() && !ASD->isStatic(); } -PrintOptions PrintOptions::printSwiftInterfaceFile(bool preferTypeRepr, +PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *M, + bool preferTypeRepr, bool printFullConvention, bool printSPIs) { PrintOptions result; @@ -115,6 +116,7 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(bool preferTypeRepr, result.PrintLongAttrsOnSeparateLines = true; result.TypeDefinitions = true; result.PrintIfConfig = false; + result.CurrentModule = M; result.FullyQualifiedTypes = true; result.UseExportedModuleNames = true; result.AllowNullTypes = false; @@ -3648,6 +3650,7 @@ class TypePrinter : public TypeVisitor { ASTPrinter &Printer; const PrintOptions &Options; + Optional> VisibleClangModules; void printGenericArgs(ArrayRef Args) { if (Args.empty()) @@ -3700,10 +3703,71 @@ class TypePrinter : public TypeVisitor { return T->hasSimpleTypeRepr(); } + /// Add all Clang modules that have been imported into `Mod` to + /// `clangModules`. + /// + /// This also takes into account any modules that are imported transitively + /// through public (`@_exported`) imports. + /// `clangModules` is a map from each imported Clang module to the + /// corresponding Swift module. + static void getImportedClangModules( + ModuleDecl *Mod, ModuleDecl::ImportFilter filter, + llvm::DenseMap &clangModules) { + SmallVector imports; + Mod->getImportedModules(imports, filter); + for (const auto &import : imports) { + if (const clang::Module *clangModule = + import.importedModule->findUnderlyingClangModule()) + clangModules[clangModule] = import.importedModule; + getImportedClangModules(import.importedModule, + ModuleDecl::ImportFilterKind::Public, + clangModules); + } + } + + /// Returns all Clang modules that are visible from `Options.CurrentModule`. + /// The returned map associates each visible Clang module with the + /// corresponding Swift module. + const llvm::DenseMap & + getVisibleClangModules() { + assert(Options.CurrentModule && + "CurrentModule needs to be set to determine imported Clang modules"); + if (!VisibleClangModules) { + VisibleClangModules.emplace(); + ModuleDecl::ImportFilter Filter = ModuleDecl::ImportFilterKind::Public; + Filter |= ModuleDecl::ImportFilterKind::Private; + getImportedClangModules(Options.CurrentModule, Filter, + *VisibleClangModules); + } + return *VisibleClangModules; + } + template void printModuleContext(T *Ty) { FileUnit *File = cast(Ty->getDecl()->getModuleScopeContext()); ModuleDecl *Mod = File->getParentModule(); + std::string ExportedModuleName = File->getExportedModuleName(); + + // Clang declarations need special treatment: Multiple Clang modules can + // contain the same declarations from a textually included header, but not + // all of these modules may be visible. We therefore need to make sure we + // choose a module that is visible from the current module. This is + // obviously only possible, however, if we know what the current module is. + const clang::Decl *ClangDecl = Ty->getDecl()->getClangDecl(); + if (ClangDecl && Options.CurrentModule) { + for (auto *Redecl : ClangDecl->redecls()) { + clang::Module *ClangModule = Redecl->getOwningModule(); + if (!ClangModule) + continue; + + if (ModuleDecl *VisibleModule = + getVisibleClangModules().lookup(ClangModule)) { + Mod = VisibleModule; + ExportedModuleName = ClangModule->ExportAsModule; + break; + } + } + } if (Options.MapCrossImportOverlaysToDeclaringModule) { if (ModuleDecl *Declaring = Mod->getDeclaringModuleIfCrossImportOverlay()) @@ -3711,8 +3775,9 @@ class TypePrinter : public TypeVisitor { } Identifier Name = Mod->getName(); - if (Options.UseExportedModuleNames) - Name = Mod->getASTContext().getIdentifier(File->getExportedModuleName()); + if (Options.UseExportedModuleNames && !ExportedModuleName.empty()) { + Name = Mod->getASTContext().getIdentifier(ExportedModuleName); + } Printer.printModuleRef(Mod, Name); Printer << "."; diff --git a/lib/Frontend/ModuleInterfaceSupport.cpp b/lib/Frontend/ModuleInterfaceSupport.cpp index ea04e971aef0a..a3c1f14f5a54a 100644 --- a/lib/Frontend/ModuleInterfaceSupport.cpp +++ b/lib/Frontend/ModuleInterfaceSupport.cpp @@ -517,7 +517,7 @@ bool swift::emitSwiftInterface(raw_ostream &out, printImports(out, Opts, M); const PrintOptions printOptions = PrintOptions::printSwiftInterfaceFile( - Opts.PreserveTypesAsWritten, Opts.PrintFullConvention, Opts.PrintSPIs); + M, Opts.PreserveTypesAsWritten, Opts.PrintFullConvention, Opts.PrintSPIs); InheritedProtocolCollector::PerTypeMap inheritedProtocolMap; SmallVector topLevelDecls; From c41530170887fb4c45ecbbf804396da2c54c5f4e Mon Sep 17 00:00:00 2001 From: Martin Boehme Date: Fri, 19 Jun 2020 12:45:09 +0200 Subject: [PATCH 03/11] Give test a more suitable name and move it to a separate directory. Putting the test in a separate directory makes it clearer that all of the input files are related and create a particular test case interaction. --- test/Interop/C/modules/Inputs/MainModule.swift | 3 --- test/Interop/C/modules/Inputs/foreign-a.h | 1 - test/Interop/C/modules/Inputs/foreign-b.h | 1 - test/Interop/C/modules/Inputs/module.modulemap | 7 ------- test/Interop/C/modules/Inputs/textual-header.h | 1 - .../Inputs/SubModule.swift | 0 test/Interop/C/modules/repro.swift | 5 ----- 7 files changed, 18 deletions(-) delete mode 100644 test/Interop/C/modules/Inputs/MainModule.swift delete mode 100644 test/Interop/C/modules/Inputs/foreign-a.h delete mode 100644 test/Interop/C/modules/Inputs/foreign-b.h delete mode 100644 test/Interop/C/modules/Inputs/module.modulemap delete mode 100644 test/Interop/C/modules/Inputs/textual-header.h rename test/Interop/C/modules/{ => print-qualified-clang-types}/Inputs/SubModule.swift (100%) delete mode 100644 test/Interop/C/modules/repro.swift diff --git a/test/Interop/C/modules/Inputs/MainModule.swift b/test/Interop/C/modules/Inputs/MainModule.swift deleted file mode 100644 index e4f4d9aef3691..0000000000000 --- a/test/Interop/C/modules/Inputs/MainModule.swift +++ /dev/null @@ -1,3 +0,0 @@ -import SubModule - -public func funcTakingForeignStruct(_ param: ForeignStruct) {} diff --git a/test/Interop/C/modules/Inputs/foreign-a.h b/test/Interop/C/modules/Inputs/foreign-a.h deleted file mode 100644 index ca297206959cc..0000000000000 --- a/test/Interop/C/modules/Inputs/foreign-a.h +++ /dev/null @@ -1 +0,0 @@ -#include "textual-header.h" diff --git a/test/Interop/C/modules/Inputs/foreign-b.h b/test/Interop/C/modules/Inputs/foreign-b.h deleted file mode 100644 index ca297206959cc..0000000000000 --- a/test/Interop/C/modules/Inputs/foreign-b.h +++ /dev/null @@ -1 +0,0 @@ -#include "textual-header.h" diff --git a/test/Interop/C/modules/Inputs/module.modulemap b/test/Interop/C/modules/Inputs/module.modulemap deleted file mode 100644 index 6e3bd2f43ad45..0000000000000 --- a/test/Interop/C/modules/Inputs/module.modulemap +++ /dev/null @@ -1,7 +0,0 @@ -module ForeignA { - header "foreign-a.h" -} - -module ForeignB { - header "foreign-b.h" -} diff --git a/test/Interop/C/modules/Inputs/textual-header.h b/test/Interop/C/modules/Inputs/textual-header.h deleted file mode 100644 index e99c9349c57a9..0000000000000 --- a/test/Interop/C/modules/Inputs/textual-header.h +++ /dev/null @@ -1 +0,0 @@ -struct ForeignStruct {}; diff --git a/test/Interop/C/modules/Inputs/SubModule.swift b/test/Interop/C/modules/print-qualified-clang-types/Inputs/SubModule.swift similarity index 100% rename from test/Interop/C/modules/Inputs/SubModule.swift rename to test/Interop/C/modules/print-qualified-clang-types/Inputs/SubModule.swift diff --git a/test/Interop/C/modules/repro.swift b/test/Interop/C/modules/repro.swift deleted file mode 100644 index a556d9ad8f714..0000000000000 --- a/test/Interop/C/modules/repro.swift +++ /dev/null @@ -1,5 +0,0 @@ -// RUN: %empty-directory(%t) -// RUN: mkdir %t/sub_module %t/main_module -// RUN: %target-swift-frontend -enable-library-evolution -swift-version 5 -emit-module -o %t/sub_module/SubModule.swiftmodule %S/Inputs/SubModule.swift -I %S/Inputs -// RUN: %target-swift-frontend -enable-library-evolution -swift-version 5 -emit-module -o %t/main_module/MainModule.swiftmodule -emit-module-interface-path %t/main_module/MainModule.swiftinterface -I %t/sub_module %S/Inputs/MainModule.swift -I %S/Inputs -// RUN: %target-swift-frontend -typecheck -swift-version 5 %t/main_module/MainModule.swiftinterface -I %t/sub_module -I %S/Inputs From 63c293e29fc2c7f7c5a58f03a0d57c565928dcab Mon Sep 17 00:00:00 2001 From: Martin Boehme Date: Fri, 19 Jun 2020 13:13:46 +0200 Subject: [PATCH 04/11] Rename SubModule to HelperModule. --- .../modules/print-qualified-clang-types/Inputs/SubModule.swift | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 test/Interop/C/modules/print-qualified-clang-types/Inputs/SubModule.swift diff --git a/test/Interop/C/modules/print-qualified-clang-types/Inputs/SubModule.swift b/test/Interop/C/modules/print-qualified-clang-types/Inputs/SubModule.swift deleted file mode 100644 index 38cd0e7b41e91..0000000000000 --- a/test/Interop/C/modules/print-qualified-clang-types/Inputs/SubModule.swift +++ /dev/null @@ -1,2 +0,0 @@ -import ForeignA -@_exported import ForeignB From 1a487e0b7b9d072d452262e525a1eba1dfa3f010 Mon Sep 17 00:00:00 2001 From: Martin Boehme Date: Fri, 19 Jun 2020 17:20:12 +0200 Subject: [PATCH 05/11] Handle Clang submodules correctly. --- lib/AST/ASTPrinter.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index 95f5ec0dbd720..9f68d5f9b3f3c 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -3716,9 +3716,10 @@ class TypePrinter : public TypeVisitor { SmallVector imports; Mod->getImportedModules(imports, filter); for (const auto &import : imports) { + ModuleDecl *importedModule = import.importedModule->getTopLevelModule(); if (const clang::Module *clangModule = - import.importedModule->findUnderlyingClangModule()) - clangModules[clangModule] = import.importedModule; + importedModule->findUnderlyingClangModule()) + clangModules[clangModule] = importedModule; getImportedClangModules(import.importedModule, ModuleDecl::ImportFilterKind::Public, clangModules); @@ -3756,7 +3757,8 @@ class TypePrinter : public TypeVisitor { const clang::Decl *ClangDecl = Ty->getDecl()->getClangDecl(); if (ClangDecl && Options.CurrentModule) { for (auto *Redecl : ClangDecl->redecls()) { - clang::Module *ClangModule = Redecl->getOwningModule(); + clang::Module *ClangModule = + Redecl->getOwningModule()->getTopLevelModule(); if (!ClangModule) continue; From ed8798cfe6e4e26868928c136ec63d20f6b65e55 Mon Sep 17 00:00:00 2001 From: Martin Boehme Date: Mon, 22 Jun 2020 08:10:38 +0200 Subject: [PATCH 06/11] Use an explicit stack instead of recursion to compute visible Clang modules. Also, make sure we only process a module once. --- lib/AST/ASTPrinter.cpp | 72 ++++++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 28 deletions(-) diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index 9f68d5f9b3f3c..bf99394f6aa20 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -3681,7 +3681,7 @@ class TypePrinter : public TypeVisitor { } } - /// Determinee whether the given type has a simple representation + /// Determine whether the given type has a simple representation /// under the current print options. bool isSimpleUnderPrintOptions(Type T) { if (auto typealias = dyn_cast(T.getPointer())) { @@ -3703,42 +3703,58 @@ class TypePrinter : public TypeVisitor { return T->hasSimpleTypeRepr(); } - /// Add all Clang modules that have been imported into `Mod` to - /// `clangModules`. - /// - /// This also takes into account any modules that are imported transitively - /// through public (`@_exported`) imports. - /// `clangModules` is a map from each imported Clang module to the - /// corresponding Swift module. - static void getImportedClangModules( - ModuleDecl *Mod, ModuleDecl::ImportFilter filter, - llvm::DenseMap &clangModules) { - SmallVector imports; - Mod->getImportedModules(imports, filter); - for (const auto &import : imports) { - ModuleDecl *importedModule = import.importedModule->getTopLevelModule(); - if (const clang::Module *clangModule = - importedModule->findUnderlyingClangModule()) - clangModules[clangModule] = importedModule; - getImportedClangModules(import.importedModule, - ModuleDecl::ImportFilterKind::Public, - clangModules); + /// Computes the map that is cached by `getVisibleClangModules()`. + /// Do not call directly. + llvm::DenseMap + computeVisibleClangModules() { + assert(Options.CurrentModule && + "CurrentModule needs to be set to determine imported Clang modules"); + + llvm::DenseMap Result; + + // For the current module, consider both private and public imports. + ModuleDecl::ImportFilter Filter = ModuleDecl::ImportFilterKind::Public; + Filter |= ModuleDecl::ImportFilterKind::Private; + SmallVector Imports; + Options.CurrentModule->getImportedModules(Imports, Filter); + + SmallVector ModulesToProcess; + for (const auto &Import : Imports) { + ModulesToProcess.push_back(Import.importedModule); + } + + SmallPtrSet Processed; + while (!ModulesToProcess.empty()) { + ModuleDecl *Mod = ModulesToProcess.back(); + ModulesToProcess.pop_back(); + + if (!Processed.insert(Mod).second) + continue; + + if (const clang::Module *ClangModule = Mod->findUnderlyingClangModule()) + Result[ClangModule] = Mod; + + // For transitive imports, consider only public imports. + Imports.clear(); + Mod->getImportedModules(Imports, ModuleDecl::ImportFilterKind::Public); + for (const auto &Import : Imports) { + ModulesToProcess.push_back(Import.importedModule); + } } + + return Result; } /// Returns all Clang modules that are visible from `Options.CurrentModule`. + /// This includes any modules that are imported transitively through public + /// (`@_exported`) imports. + /// /// The returned map associates each visible Clang module with the /// corresponding Swift module. const llvm::DenseMap & getVisibleClangModules() { - assert(Options.CurrentModule && - "CurrentModule needs to be set to determine imported Clang modules"); if (!VisibleClangModules) { - VisibleClangModules.emplace(); - ModuleDecl::ImportFilter Filter = ModuleDecl::ImportFilterKind::Public; - Filter |= ModuleDecl::ImportFilterKind::Private; - getImportedClangModules(Options.CurrentModule, Filter, - *VisibleClangModules); + VisibleClangModules = computeVisibleClangModules(); } return *VisibleClangModules; } From 86209c7070250e9ccac32731756b3c9cc4a40ab2 Mon Sep 17 00:00:00 2001 From: Martin Boehme Date: Mon, 22 Jun 2020 08:38:23 +0200 Subject: [PATCH 07/11] Various responses to review comments. --- include/swift/AST/PrintOptions.h | 2 +- lib/AST/ASTPrinter.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/swift/AST/PrintOptions.h b/include/swift/AST/PrintOptions.h index 22537e677ffcc..61f54478e2ff9 100644 --- a/include/swift/AST/PrintOptions.h +++ b/include/swift/AST/PrintOptions.h @@ -550,7 +550,7 @@ struct PrintOptions { /// attributes. /// /// \see swift::emitSwiftInterface - static PrintOptions printSwiftInterfaceFile(ModuleDecl *M, + static PrintOptions printSwiftInterfaceFile(ModuleDecl *ModuleToPrint, bool preferTypeRepr, bool printFullConvention, bool printSPIs); diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index bf99394f6aa20..87f8f023e7321 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -107,7 +107,7 @@ static bool contributesToParentTypeStorage(const AbstractStorageDecl *ASD) { return !ND->isResilient() && ASD->hasStorage() && !ASD->isStatic(); } -PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *M, +PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint, bool preferTypeRepr, bool printFullConvention, bool printSPIs) { @@ -116,7 +116,7 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *M, result.PrintLongAttrsOnSeparateLines = true; result.TypeDefinitions = true; result.PrintIfConfig = false; - result.CurrentModule = M; + result.CurrentModule = ModuleToPrint; result.FullyQualifiedTypes = true; result.UseExportedModuleNames = true; result.AllowNullTypes = false; @@ -3768,8 +3768,8 @@ class TypePrinter : public TypeVisitor { // Clang declarations need special treatment: Multiple Clang modules can // contain the same declarations from a textually included header, but not // all of these modules may be visible. We therefore need to make sure we - // choose a module that is visible from the current module. This is - // obviously only possible, however, if we know what the current module is. + // choose a module that is visible from the current module. This is possible + // only if we know what the current module is. const clang::Decl *ClangDecl = Ty->getDecl()->getClangDecl(); if (ClangDecl && Options.CurrentModule) { for (auto *Redecl : ClangDecl->redecls()) { From 30ad2ef698adda300cfd4f5093b8989714420c44 Mon Sep 17 00:00:00 2001 From: Martin Boehme Date: Mon, 22 Jun 2020 09:33:44 +0200 Subject: [PATCH 08/11] Formatting fix. --- lib/AST/ASTPrinter.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index 87f8f023e7321..3b1d570950399 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -3650,7 +3650,8 @@ class TypePrinter : public TypeVisitor { ASTPrinter &Printer; const PrintOptions &Options; - Optional> VisibleClangModules; + Optional> + VisibleClangModules; void printGenericArgs(ArrayRef Args) { if (Args.empty()) From 9973f627a536a76a6fba43ebd23fd25c28f25b97 Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Fri, 4 Dec 2020 14:53:31 +0100 Subject: [PATCH 09/11] Sync main --- lib/AST/ASTPrinter.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index 3b1d570950399..de90242166e2e 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -3714,9 +3714,9 @@ class TypePrinter : public TypeVisitor { llvm::DenseMap Result; // For the current module, consider both private and public imports. - ModuleDecl::ImportFilter Filter = ModuleDecl::ImportFilterKind::Public; - Filter |= ModuleDecl::ImportFilterKind::Private; - SmallVector Imports; + ModuleDecl::ImportFilter Filter = ModuleDecl::ImportFilterKind::Exported; + Filter |= ModuleDecl::ImportFilterKind::Default; + SmallVector Imports; Options.CurrentModule->getImportedModules(Imports, Filter); SmallVector ModulesToProcess; @@ -3737,7 +3737,7 @@ class TypePrinter : public TypeVisitor { // For transitive imports, consider only public imports. Imports.clear(); - Mod->getImportedModules(Imports, ModuleDecl::ImportFilterKind::Public); + Mod->getImportedModules(Imports, ModuleDecl::ImportFilterKind::Exported); for (const auto &Import : Imports) { ModulesToProcess.push_back(Import.importedModule); } @@ -3764,7 +3764,7 @@ class TypePrinter : public TypeVisitor { void printModuleContext(T *Ty) { FileUnit *File = cast(Ty->getDecl()->getModuleScopeContext()); ModuleDecl *Mod = File->getParentModule(); - std::string ExportedModuleName = File->getExportedModuleName(); + StringRef ExportedModuleName = File->getExportedModuleName(); // Clang declarations need special treatment: Multiple Clang modules can // contain the same declarations from a textually included header, but not From 7b8e6ac0bee6468428ab60c9f8d997d2f4a1a645 Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Thu, 10 Dec 2020 13:33:43 +0100 Subject: [PATCH 10/11] Add test for importing @_spi module --- lib/AST/ASTPrinter.cpp | 1 + .../Inputs/SpiMainModule.swift | 3 +++ .../Inputs/module.modulemap | 10 ++-------- .../Inputs/textual-header.h | 2 +- .../print-qualified-clang-types-spi.swift | 14 ++++++++++++++ 5 files changed, 21 insertions(+), 9 deletions(-) create mode 100644 test/Interop/C/modules/print-qualified-clang-types/Inputs/SpiMainModule.swift create mode 100644 test/Interop/C/modules/print-qualified-clang-types/print-qualified-clang-types-spi.swift diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index de90242166e2e..9372159102d0b 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -3716,6 +3716,7 @@ class TypePrinter : public TypeVisitor { // For the current module, consider both private and public imports. ModuleDecl::ImportFilter Filter = ModuleDecl::ImportFilterKind::Exported; Filter |= ModuleDecl::ImportFilterKind::Default; + Filter |= ModuleDecl::ImportFilterKind::SPIAccessControl; SmallVector Imports; Options.CurrentModule->getImportedModules(Imports, Filter); diff --git a/test/Interop/C/modules/print-qualified-clang-types/Inputs/SpiMainModule.swift b/test/Interop/C/modules/print-qualified-clang-types/Inputs/SpiMainModule.swift new file mode 100644 index 0000000000000..2980e40a28450 --- /dev/null +++ b/test/Interop/C/modules/print-qualified-clang-types/Inputs/SpiMainModule.swift @@ -0,0 +1,3 @@ +@_spi(dummy) import HelperModule + +public func funcTakingForeignStruct(_ param: ForeignStruct) {} diff --git a/test/Interop/C/modules/print-qualified-clang-types/Inputs/module.modulemap b/test/Interop/C/modules/print-qualified-clang-types/Inputs/module.modulemap index bc5f5f4f50bab..6e3bd2f43ad45 100644 --- a/test/Interop/C/modules/print-qualified-clang-types/Inputs/module.modulemap +++ b/test/Interop/C/modules/print-qualified-clang-types/Inputs/module.modulemap @@ -1,13 +1,7 @@ module ForeignA { - // Nest the header in a sub-module to make sure these are handled correctly. - module Sub { - header "foreign-a.h" - } + header "foreign-a.h" } module ForeignB { - // Nest the header in a sub-module to make sure these are handled correctly. - module Sub { - header "foreign-b.h" - } + header "foreign-b.h" } diff --git a/test/Interop/C/modules/print-qualified-clang-types/Inputs/textual-header.h b/test/Interop/C/modules/print-qualified-clang-types/Inputs/textual-header.h index a53012248d706..e99c9349c57a9 100644 --- a/test/Interop/C/modules/print-qualified-clang-types/Inputs/textual-header.h +++ b/test/Interop/C/modules/print-qualified-clang-types/Inputs/textual-header.h @@ -1 +1 @@ -typedef struct {} ForeignStruct; +struct ForeignStruct {}; diff --git a/test/Interop/C/modules/print-qualified-clang-types/print-qualified-clang-types-spi.swift b/test/Interop/C/modules/print-qualified-clang-types/print-qualified-clang-types-spi.swift new file mode 100644 index 0000000000000..d3481e2505eb4 --- /dev/null +++ b/test/Interop/C/modules/print-qualified-clang-types/print-qualified-clang-types-spi.swift @@ -0,0 +1,14 @@ +// This tests verifies that SPI visible modules are understood by the machinery +// that produces .swiftinterface files. +// +// See the documentation for the print-qualified-clang-types.swift test next to +// this one for more context. + +// RUN: %empty-directory(%t) +// RUN: mkdir %t/helper_module %t/spi_main_module +// 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 +// 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 +// RUN: %FileCheck --input-file=%t/spi_main_module/SpiMainModule.swiftinterface %s +// RUN: %target-swift-frontend -typecheck -swift-version 5 %t/spi_main_module/SpiMainModule.swiftinterface -I %t/helper_module -I %S/Inputs + +// CHECK: public func funcTakingForeignStruct(_ param: ForeignB.ForeignStruct) From 2134b8f2728f19f09327468a582b2c817e2559b6 Mon Sep 17 00:00:00 2001 From: Rosica Dejanovska Date: Thu, 10 Dec 2020 15:58:53 +0100 Subject: [PATCH 11/11] Enable test that failed without this PR --- .../print-qualified-clang-types.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/Interop/C/modules/print-qualified-clang-types/print-qualified-clang-types.swift b/test/Interop/C/modules/print-qualified-clang-types/print-qualified-clang-types.swift index a14102af9b264..13a78745481e8 100644 --- a/test/Interop/C/modules/print-qualified-clang-types/print-qualified-clang-types.swift +++ b/test/Interop/C/modules/print-qualified-clang-types/print-qualified-clang-types.swift @@ -34,5 +34,3 @@ // RUN: %target-swift-frontend -typecheck -swift-version 5 %t/main_module/MainModule.swiftinterface -I %t/helper_module -I %S/Inputs // CHECK: public func funcTakingForeignStruct(_ param: ForeignB.ForeignStruct) - -// REQUIRES: SR-13032