From fb155133a3386279d1f57f485785761870676f84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Laferri=C3=A8re?= Date: Mon, 18 Apr 2022 12:46:58 -0700 Subject: [PATCH 1/3] [Sema][NFC] Refactor the logic matching paths in the SDK --- lib/AST/Module.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index 2ec065cf45b0a..7275528614468 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -2629,17 +2629,24 @@ LibraryLevel ModuleLibraryLevelRequest::evaluate(Evaluator &evaluator, const ModuleDecl *module) const { auto &ctx = module->getASTContext(); + namespace path = llvm::sys::path; + SmallString<128> scratch; + + /// Is \p path under the folder SDK/a/b/c/d/e? + auto hasSDKPrefix = + [&](StringRef path, const Twine &a, const Twine &b = "", + const Twine &c = "", const Twine &d = "", const Twine &e = "") { + scratch = ctx.SearchPathOpts.getSDKPath(); + path::append(scratch, a, b, c, d); + path::append(scratch, e); + return path.startswith(scratch); + }; /// Is \p modulePath from System/Library/PrivateFrameworks/? auto fromPrivateFrameworks = [&](StringRef modulePath) -> bool { if (!ctx.LangOpts.Target.isOSDarwin()) return false; - namespace path = llvm::sys::path; - SmallString<128> scratch; - scratch = ctx.SearchPathOpts.getSDKPath(); - path::append(scratch, "System", "Library", "PrivateFrameworks"); - return hasPrefix(path::begin(modulePath), path::end(modulePath), - path::begin(scratch), path::end(scratch)); + return hasSDKPrefix(modulePath, "System", "Library", "PrivateFrameworks"); }; if (module->isNonSwiftModule()) { From 6cc2399ddbaeb0ea9af98eb1e27520c8a8f3fca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Laferri=C3=A8re?= Date: Mon, 18 Apr 2022 12:50:38 -0700 Subject: [PATCH 2/3] [Sema] delete superfluous test about errors on imports of public modules --- ...tion-only-import-suggestion-as-error.swift | 29 ------------------- ...mplementation-only-import-suggestion.swift | 8 ++--- 2 files changed, 4 insertions(+), 33 deletions(-) delete mode 100644 test/Sema/implementation-only-import-suggestion-as-error.swift diff --git a/test/Sema/implementation-only-import-suggestion-as-error.swift b/test/Sema/implementation-only-import-suggestion-as-error.swift deleted file mode 100644 index d75eaa031e7af..0000000000000 --- a/test/Sema/implementation-only-import-suggestion-as-error.swift +++ /dev/null @@ -1,29 +0,0 @@ -/// Same test as implementation-only-import-suggestion upgrading warnings to -/// errors. We can remove this test when this becomes the default behavior. - -// RUN: %empty-directory(%t) -// REQUIRES: VENDOR=apple - -/// Prepare the SDK. -// RUN: cp -r %S/Inputs/public-private-sdk %t/sdk -// RUN: %target-swift-frontend -emit-module -module-name PublicSwift \ -// RUN: %t/sdk/System/Library/Frameworks/PublicSwift.framework/Modules/PublicSwift.swiftmodule/source.swift \ -// RUN: -o %t/sdk/System/Library/Frameworks/PublicSwift.framework/Modules/PublicSwift.swiftmodule/%target-swiftmodule-name -// RUN: %target-swift-frontend -emit-module -module-name PrivateSwift \ -// RUN: %t/sdk/System/Library/PrivateFrameworks/PrivateSwift.framework/Modules/PrivateSwift.swiftmodule/source.swift \ -// RUN: -o %t/sdk/System/Library/PrivateFrameworks/PrivateSwift.framework/Modules/PrivateSwift.swiftmodule/%target-swiftmodule-name - -/// Expect errors when building a public client. -// RUN: env ENABLE_PUBLIC_IMPORT_OF_PRIVATE_AS_ERROR=1 \ -// RUN: %target-swift-frontend -typecheck -sdk %t/sdk -module-cache-path %t %s \ -// RUN: -F %t/sdk/System/Library/PrivateFrameworks/ \ -// RUN: -library-level api -verify -module-name MainLib - -import PublicSwift -import PrivateSwift // expected-error{{private module 'PrivateSwift' is imported publicly from the public module 'MainLib'}} - -import PublicClang -import PublicClang_Private // expected-error{{private module 'PublicClang_Private' is imported publicly from the public module 'MainLib'}} -import FullyPrivateClang // expected-error{{private module 'FullyPrivateClang' is imported publicly from the public module 'MainLib'}} - -@_exported import MainLib // expected-warning{{private module 'MainLib' is imported publicly from the public module 'MainLib'}} diff --git a/test/Sema/implementation-only-import-suggestion.swift b/test/Sema/implementation-only-import-suggestion.swift index 890e736700a3d..4b635a7004d6a 100644 --- a/test/Sema/implementation-only-import-suggestion.swift +++ b/test/Sema/implementation-only-import-suggestion.swift @@ -11,12 +11,12 @@ // RUN: %t/sdk/System/Library/PrivateFrameworks/PrivateSwift.framework/Modules/PrivateSwift.swiftmodule/source.swift \ // RUN: -o %t/sdk/System/Library/PrivateFrameworks/PrivateSwift.framework/Modules/PrivateSwift.swiftmodule/%target-swiftmodule-name -/// Expect warnings when building a public client. +/// Expect errors when building a public client. // RUN: %target-swift-frontend -typecheck -sdk %t/sdk -module-cache-path %t %s \ // RUN: -F %t/sdk/System/Library/PrivateFrameworks/ \ // RUN: -library-level api -verify -D PUBLIC_IMPORTS -module-name MainLib -/// Expect no warnings when building an SPI client. +/// Expect no errors when building an SPI client. // RUN: %target-swift-frontend -typecheck -sdk %t/sdk -module-cache-path %t %s \ // RUN: -F %t/sdk/System/Library/PrivateFrameworks/ \ // RUN: -library-level spi -D PUBLIC_IMPORTS -module-name MainLib @@ -26,7 +26,7 @@ // RUN: -F %t/sdk/System/Library/PrivateFrameworks/ \ // RUN: -library-level spi -D PUBLIC_IMPORTS -module-name MainLib -/// Expect no warnings when building a client with some other library level. +/// Expect no errors when building a client with some other library level. // RUN: %target-swift-frontend -typecheck -sdk %t/sdk -module-cache-path %t %s \ // RUN: -F %t/sdk/System/Library/PrivateFrameworks/ \ // RUN: -D PUBLIC_IMPORTS -module-name MainLib @@ -42,7 +42,7 @@ import PublicClang_Private // expected-error{{private module 'PublicClang_Privat import FullyPrivateClang // expected-error{{private module 'FullyPrivateClang' is imported publicly from the public module 'MainLib'}} @_exported import MainLib // expected-warning{{private module 'MainLib' is imported publicly from the public module 'MainLib'}} -/// Expect no warnings with implementation-only imports. +/// Expect no errors with implementation-only imports. // RUN: %target-swift-frontend -typecheck -sdk %t/sdk -module-cache-path %t %s \ // RUN: -F %t/sdk/System/Library/PrivateFrameworks/ \ // RUN: -library-level api -D IMPL_ONLY_IMPORTS From 91001f0257c66e38fedca7464a355791b7f9f61a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Laferri=C3=A8re?= Date: Mon, 18 Apr 2022 14:24:12 -0700 Subject: [PATCH 3/3] [Sema] Consider modules imported from more paths as private Consider both frameworks and free floating modules imported from more private path as SPI. This will make the compiler raise errors on public imports of more private modules. rdar://91904154 --- lib/AST/Module.cpp | 8 +++++--- .../public-private-sdk/usr/local/include/LocalClang.h | 1 + .../public-private-sdk/usr/local/include/module.modulemap | 4 ++++ test/Sema/implementation-only-import-suggestion.swift | 2 ++ 4 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 test/Sema/Inputs/public-private-sdk/usr/local/include/LocalClang.h create mode 100644 test/Sema/Inputs/public-private-sdk/usr/local/include/module.modulemap diff --git a/lib/AST/Module.cpp b/lib/AST/Module.cpp index 7275528614468..2f26d97c6f6b3 100644 --- a/lib/AST/Module.cpp +++ b/lib/AST/Module.cpp @@ -2646,7 +2646,10 @@ ModuleLibraryLevelRequest::evaluate(Evaluator &evaluator, auto fromPrivateFrameworks = [&](StringRef modulePath) -> bool { if (!ctx.LangOpts.Target.isOSDarwin()) return false; - return hasSDKPrefix(modulePath, "System", "Library", "PrivateFrameworks"); + return hasSDKPrefix(modulePath, "AppleInternal", "Library", "Frameworks") || + hasSDKPrefix(modulePath, "System", "Library", "PrivateFrameworks") || + hasSDKPrefix(modulePath, "System", "iOSSupport", "System", "Library", "PrivateFrameworks") || + hasSDKPrefix(modulePath, "usr", "local", "include"); }; if (module->isNonSwiftModule()) { @@ -2654,8 +2657,7 @@ ModuleLibraryLevelRequest::evaluate(Evaluator &evaluator, // Imported clangmodules are SPI if they are defined by a private // modulemap or from the PrivateFrameworks folder in the SDK. bool moduleIsSPI = underlying->ModuleMapIsPrivate || - (underlying->isPartOfFramework() && - fromPrivateFrameworks(underlying->PresumedModuleMapFile)); + fromPrivateFrameworks(underlying->PresumedModuleMapFile); return moduleIsSPI ? LibraryLevel::SPI : LibraryLevel::API; } return LibraryLevel::Other; diff --git a/test/Sema/Inputs/public-private-sdk/usr/local/include/LocalClang.h b/test/Sema/Inputs/public-private-sdk/usr/local/include/LocalClang.h new file mode 100644 index 0000000000000..d15abba59766b --- /dev/null +++ b/test/Sema/Inputs/public-private-sdk/usr/local/include/LocalClang.h @@ -0,0 +1 @@ +// Empty diff --git a/test/Sema/Inputs/public-private-sdk/usr/local/include/module.modulemap b/test/Sema/Inputs/public-private-sdk/usr/local/include/module.modulemap new file mode 100644 index 0000000000000..98ebc5b08218a --- /dev/null +++ b/test/Sema/Inputs/public-private-sdk/usr/local/include/module.modulemap @@ -0,0 +1,4 @@ +module LocalClang [system] { + header "LocalClang.h" + export * +} diff --git a/test/Sema/implementation-only-import-suggestion.swift b/test/Sema/implementation-only-import-suggestion.swift index 4b635a7004d6a..a7e3bc354cddf 100644 --- a/test/Sema/implementation-only-import-suggestion.swift +++ b/test/Sema/implementation-only-import-suggestion.swift @@ -40,6 +40,7 @@ import PrivateSwift // expected-error{{private module 'PrivateSwift' is imported import PublicClang import PublicClang_Private // expected-error{{private module 'PublicClang_Private' is imported publicly from the public module 'MainLib'}} import FullyPrivateClang // expected-error{{private module 'FullyPrivateClang' is imported publicly from the public module 'MainLib'}} +import LocalClang // expected-error{{private module 'LocalClang' is imported publicly from the public module 'MainLib'}} @_exported import MainLib // expected-warning{{private module 'MainLib' is imported publicly from the public module 'MainLib'}} /// Expect no errors with implementation-only imports. @@ -51,6 +52,7 @@ import FullyPrivateClang // expected-error{{private module 'FullyPrivateClang' i @_implementationOnly import PrivateSwift @_implementationOnly import PublicClang_Private @_implementationOnly import FullyPrivateClang +@_implementationOnly import LocalClang #endif