diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index 044d5769dd1e5..d2f476ec27ebf 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -2971,10 +2971,10 @@ SourceFile::getImportAccessLevel(const ModuleDecl *targetModule) const { assert(targetModule != getParentModule() && "getImportAccessLevel doesn't support checking for a self-import"); - /// Order of relevancy of `import` to reach `targetModule`. - /// Lower is better/more authoritative. + /// Order of relevancy of `import` to reach `targetModule` assuming the same + /// visibility level. Lower is better/more authoritative. auto rateImport = [&](const ImportAccessLevel import) -> int { - auto importedModule = import->module.importedModule; + auto importedModule = import->module.importedModule->getTopLevelModule(); // Prioritize public names: if (targetModule->getExportAsName() == importedModule->getBaseIdentifier()) @@ -2989,9 +2989,14 @@ SourceFile::getImportAccessLevel(const ModuleDecl *targetModule) const { if (targetModule == importedModule->getUnderlyingModuleIfOverlay()) return 3; - // Any import in the sources. - if (import->importLoc.isValid()) - return 4; + // Any import in the sources: + if (import->importLoc.isValid()) { + // Prefer clang submodules to their top level modules: + if (import->module.importedModule != importedModule) + return 4; + + return 5; + } return 10; }; @@ -3001,11 +3006,12 @@ SourceFile::getImportAccessLevel(const ModuleDecl *targetModule) const { auto &imports = getASTContext().getImportCache(); ImportAccessLevel restrictiveImport = std::nullopt; for (auto &import : *Imports) { + auto importedModule = import.module.importedModule->getTopLevelModule(); if ((!restrictiveImport.has_value() || import.accessLevel > restrictiveImport->accessLevel || (import.accessLevel == restrictiveImport->accessLevel && rateImport(import) < rateImport(restrictiveImport))) && - imports.isImportedBy(targetModule, import.module.importedModule)) { + imports.isImportedBy(targetModule, importedModule)) { restrictiveImport = import; } } diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp index 3a4dbd1112107..7f4bf843a0f1c 100644 --- a/lib/Sema/ResilienceDiagnostics.cpp +++ b/lib/Sema/ResilienceDiagnostics.cpp @@ -162,7 +162,8 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc, ModuleDecl *importedVia = attributedImport.module.importedModule, *sourceModule = D->getModuleContext(); ctx.Diags.diagnose(loc, diag::module_api_import_aliases, D, importedVia, - sourceModule, importedVia == sourceModule); + sourceModule, + importedVia->getTopLevelModule() == sourceModule); }); auto ignoredDowngradeToWarning = DowngradeToWarning::No; @@ -283,7 +284,8 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D, ModuleDecl *importedVia = attributedImport.module.importedModule, *sourceModule = D->getModuleContext(); ctx.Diags.diagnose(loc, diag::module_api_import, D, importedVia, - sourceModule, importedVia == sourceModule, + sourceModule, + importedVia->getTopLevelModule() == sourceModule, /*isImplicit*/ false); } }); @@ -426,7 +428,7 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc, ctx.Diags.diagnose(loc, diag::module_api_import_conformance, rootConf->getType(), rootConf->getProtocol(), importedVia, sourceModule, - importedVia == sourceModule); + importedVia->getTopLevelModule() == sourceModule); }); auto originKind = getDisallowedOriginKind(ext, where); diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index fe19abefed39f..3f563d2fd5a62 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -2768,7 +2768,7 @@ void swift::recordRequiredImportAccessLevelForDecl(const ValueDecl *decl, *sourceModule = decl->getModuleContext(); dc->getASTContext().Diags.diagnose( diagLoc, diag::module_api_import, decl, importedVia, sourceModule, - importedVia == sourceModule, loc.isInvalid()); + importedVia->getTopLevelModule() == sourceModule, loc.isInvalid()); }); } diff --git a/test/Sema/access-level-import-inconsistent-ordering.swift b/test/Sema/access-level-import-inconsistent-ordering.swift new file mode 100644 index 0000000000000..9d57e916951fd --- /dev/null +++ b/test/Sema/access-level-import-inconsistent-ordering.swift @@ -0,0 +1,109 @@ +/// The order of imports in sources shouldn't matter for access-levels. + +// RUN: %empty-directory(%t) +// RUN: split-file %s %t --leading-lines + +/// Build the libraries. +// RUN: %target-swift-frontend -emit-module %t/Lib1.swift -module-name Lib1 -o %t +// RUN: %target-swift-frontend -emit-module %t/Lib2.swift -module-name Lib2 -o %t + +/// Test main cases. +// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t -package-name pkg \ +// RUN: -verify -verify-ignore-unrelated +// RUN: %target-swift-frontend -typecheck %t/Client_Clang.swift -I %t \ +// RUN: -verify -verify-ignore-unrelated +// RUN: %target-swift-frontend -typecheck %t/Client_Clang.swift -I %t -DINVERT \ +// RUN: -verify -verify-ignore-unrelated +// RUN: %target-swift-frontend -typecheck %t/Client_Clang_Submodules.swift -I %t \ +// RUN: -verify -verify-ignore-unrelated +// RUN: %target-swift-frontend -typecheck %t/Client_Clang_Submodules.swift -I %t -DINVERT \ +// RUN: -verify -verify-ignore-unrelated + +// REQUIRES: VENDOR=apple + +//--- Lib1.swift +public struct Type1 {} + +//--- Lib2.swift +public struct Type2 {} + +//--- Client.swift + +/// Simple public vs internal. +package import Lib1 // expected-note {{imported 'package' here}} +// expected-note @-1 {{struct 'Type1' imported as 'package' from 'Lib1' here}} +internal import Lib1 // expected-warning {{module 'Lib1' is imported as 'package' from the same file; this 'internal' access level will be ignored}} + +/// Simple package vs internal, inverted. +internal import Lib2 // expected-warning {{module 'Lib2' is imported as 'package' from the same file; this 'internal' access level will be ignored}} +package import Lib2 // expected-note {{imported 'package' here}} +// expected-note @-1 {{struct 'Type2' imported as 'package' from 'Lib2' here}} + +public func dummyAPI(t1: Type1) {} // expected-error {{function cannot be declared public because its parameter uses a package type}} +// expected-note @-1 {{struct 'Type1' is imported by this file as 'package' from 'Lib1'}} + +public func dummyAPI(t2: Type2) {} // expected-error {{function cannot be declared public because its parameter uses a package type}} +// expected-note @-1 {{struct 'Type2' is imported by this file as 'package' from 'Lib2'}} + +//--- Client_Clang.swift + +#if INVERT +private import ClangLib +#endif + +internal import ClangLib2 // expected-note {{struct 'ClangType' imported as 'internal' from 'ClangLib2' here}} + +#if !INVERT +private import ClangLib +#endif + +public func dummyAPI(t2: ClangType) {} +// expected-error @-1 {{function cannot be declared public because its parameter uses an internal type}} +// expected-note @-2 {{struct 'ClangType' is imported by this file as 'internal' from 'ClangLib2'}} + +//--- Client_Clang_Submodules.swift + +#if INVERT +private import ClangLib.Sub1 +#endif + +internal import ClangLib.Sub2 // expected-note {{struct 'SubType' imported as 'internal' from 'Sub2' here}} + +#if !INVERT +private import ClangLib.Sub1 +#endif + +public func dummyAPI(t2: SubType) {} +// expected-error @-1 {{function cannot be declared public because its parameter uses an internal type}} +// expected-note @-2 {{struct 'SubType' is imported by this file as 'internal' from 'Sub2'}} + +//--- module.modulemap + +module ClangLib { + header "ClangLib1.h" + + explicit module Sub1 { + header "Sub1.h" + } + + explicit module Sub2 { + header "Sub2.h" + } +} + +module ClangLib2 { + header "ClangLib2.h" + export * +} + +//--- ClangLib1.h +struct ClangType {}; + +//--- ClangLib2.h +#include + +//--- Sub1.h +struct SubType {}; + +//--- Sub2.h +#include "Sub1.h" diff --git a/test/Sema/superfluously-public-imports.swift b/test/Sema/superfluously-public-imports.swift index 2aa6665bb68b7..a4642d1177a78 100644 --- a/test/Sema/superfluously-public-imports.swift +++ b/test/Sema/superfluously-public-imports.swift @@ -337,8 +337,8 @@ public import ClangSubmoduleUnused.ClangSubmoduleUnsuedSubmodule // expected-war public import ClangTopModule.ClangTopModuleSubmodule public func clangUser(a: ClangSimpleType) {} // expected-remark {{struct 'ClangSimpleType' is imported via 'ClangSimple'}} -public func clangUser(a: ClangSubmoduleSubmoduleType) {} // expected-remark {{struct 'ClangSubmoduleSubmoduleType' is imported via 'ClangSubmodule'}} -public func clangUser(a: ClangTopModuleType) {} // expected-remark {{struct 'ClangTopModuleType' is imported via 'ClangTopModule'}} +public func clangUser(a: ClangSubmoduleSubmoduleType) {} // expected-remark {{struct 'ClangSubmoduleSubmoduleType' is imported via 'ClangSubmoduleSubmodule'}} +public func clangUser(a: ClangTopModuleType) {} // expected-remark {{struct 'ClangTopModuleType' is imported via 'ClangTopModuleSubmodule'}} //--- ClientOfClangReexportedSubmodules.swift public import ClangReexportedSubmodulePublic.ClangReexportedSubmodulePublicSub