Skip to content

[Sema] Consider more paths in the SDK as defining private modules #42432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2629,26 +2629,35 @@ 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, "AppleInternal", "Library", "Frameworks") ||
hasSDKPrefix(modulePath, "System", "Library", "PrivateFrameworks") ||
hasSDKPrefix(modulePath, "System", "iOSSupport", "System", "Library", "PrivateFrameworks") ||
hasSDKPrefix(modulePath, "usr", "local", "include");
};

if (module->isNonSwiftModule()) {
if (auto *underlying = module->findUnderlyingClangModule()) {
// 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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// Empty
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module LocalClang [system] {
header "LocalClang.h"
export *
}
29 changes: 0 additions & 29 deletions test/Sema/implementation-only-import-suggestion-as-error.swift

This file was deleted.

10 changes: 6 additions & 4 deletions test/Sema/implementation-only-import-suggestion.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -40,9 +40,10 @@ 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 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
Expand All @@ -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

Expand Down