Skip to content

Commit 04709c1

Browse files
authored
Merge pull request #36608 from xymus/lib-access-level
[Sema] Define library levels to detect public import of private modules and more
2 parents 71cd85c + 6bbb9f0 commit 04709c1

File tree

19 files changed

+212
-0
lines changed

19 files changed

+212
-0
lines changed

include/swift/AST/DiagnosticsFrontend.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ ERROR(error_unsupported_target_os, none,
3636
ERROR(error_unsupported_target_arch, none,
3737
"unsupported target architecture: '%0'", (StringRef))
3838

39+
ERROR(error_unknown_library_level, none,
40+
"unknown library level '%0', "
41+
"expected one of 'api', 'spi' or 'other'", (StringRef))
42+
3943
ERROR(error_unsupported_opt_for_target, none,
4044
"unsupported option '%0' for target '%1'", (StringRef, StringRef))
4145

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2843,6 +2843,9 @@ WARNING(warn_implementation_only_conflict,none,
28432843
(Identifier))
28442844
NOTE(implementation_only_conflict_here,none,
28452845
"imported as implementation-only here", ())
2846+
WARNING(warn_public_import_of_private_module,none,
2847+
"private module %0 is imported publicly from the public module %1",
2848+
(Identifier, Identifier))
28462849

28472850
ERROR(implementation_only_decl_non_override,none,
28482851
"'@_implementationOnly' can only be used on overrides", ())

include/swift/AST/Module.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ namespace swift {
5656
class FileUnit;
5757
class FuncDecl;
5858
class InfixOperatorDecl;
59+
enum class LibraryLevel : uint8_t;
5960
class LinkLibrary;
6061
class ModuleLoader;
6162
class NominalTypeDecl;
@@ -462,6 +463,9 @@ class ModuleDecl : public DeclContext, public TypeDecl {
462463
Bits.ModuleDecl.RawResilienceStrategy = unsigned(strategy);
463464
}
464465

466+
/// Distribution level of the module.
467+
LibraryLevel getLibraryLevel() const;
468+
465469
/// Returns true if this module was or is being compiled for testing.
466470
bool hasIncrementalInfo() const { return Bits.ModuleDecl.HasIncrementalInfo; }
467471
void setHasIncrementalInfo(bool enabled = true) {

include/swift/AST/TypeCheckRequests.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2753,6 +2753,24 @@ class HasImplementationOnlyImportsRequest
27532753
bool isCached() const { return true; }
27542754
};
27552755

2756+
/// Get the library level of a module.
2757+
class ModuleLibraryLevelRequest
2758+
: public SimpleRequest<ModuleLibraryLevelRequest,
2759+
LibraryLevel(const ModuleDecl *),
2760+
RequestFlags::Cached> {
2761+
public:
2762+
using SimpleRequest::SimpleRequest;
2763+
2764+
private:
2765+
friend SimpleRequest;
2766+
2767+
LibraryLevel evaluate(Evaluator &evaluator, const ModuleDecl *module) const;
2768+
2769+
public:
2770+
// Cached.
2771+
bool isCached() const { return true; }
2772+
};
2773+
27562774
class ResolveTypeRequest
27572775
: public SimpleRequest<ResolveTypeRequest,
27582776
Type(const TypeResolution *, TypeRepr *,

include/swift/AST/TypeCheckerTypeIDZone.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ SWIFT_REQUEST(TypeChecker, HasDynamicCallableAttributeRequest,
126126
bool(CanType), Cached, NoLocationInfo)
127127
SWIFT_REQUEST(TypeChecker, HasImplementationOnlyImportsRequest,
128128
bool(SourceFile *), Cached, NoLocationInfo)
129+
SWIFT_REQUEST(TypeChecker, ModuleLibraryLevelRequest,
130+
LibraryLevel(ModuleDecl *), Cached, NoLocationInfo)
129131
SWIFT_REQUEST(TypeChecker, InferredGenericSignatureRequest,
130132
GenericSignature (ModuleDecl *, const GenericSignatureImpl *,
131133
GenericParamSource,

include/swift/Basic/LangOptions.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,20 @@ namespace swift {
5555
Complete,
5656
};
5757

58+
/// Access or distribution level of a library.
59+
enum class LibraryLevel : uint8_t {
60+
/// Application Programming Interface that is publicly distributed so
61+
/// public decls are really public and only @_spi decls are SPI.
62+
API,
63+
64+
/// System Programming Interface that has restricted distribution
65+
/// all decls in the module are considered to be SPI including public ones.
66+
SPI,
67+
68+
/// The library has some other undefined distribution.
69+
Other
70+
};
71+
5872
/// A collection of options that affect the language dialect and
5973
/// provide compiler debugging facilities.
6074
class LangOptions final {
@@ -306,6 +320,9 @@ namespace swift {
306320
/// Objective-C-derived classes and 'dynamic' members.
307321
bool EnableSwift3ObjCInference = false;
308322

323+
/// Access or distribution level of the whole module being parsed.
324+
LibraryLevel LibraryLevel = LibraryLevel::Other;
325+
309326
/// Warn about cases where Swift 3 would infer @objc but later versions
310327
/// of Swift do not.
311328
Swift3ObjCInferenceWarnings WarnSwift3ObjCInference =

include/swift/Option/FrontendOptions.td

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,11 @@ def disable_swift3_objc_inference :
433433
Flags<[FrontendOption, HelpHidden]>,
434434
HelpText<"Disable Swift 3's @objc inference rules for NSObject-derived classes and 'dynamic' members (emulates Swift 4 behavior)">;
435435

436+
def library_level : Separate<["-"], "library-level">,
437+
MetaVarName<"<level>">,
438+
Flags<[FrontendOption]>,
439+
HelpText<"Library distribution level 'api', 'spi' or 'other' (the default)">;
440+
436441
def enable_implicit_dynamic : Flag<["-"], "enable-implicit-dynamic">,
437442
Flags<[FrontendOption, NoInteractiveOption, HelpHidden]>,
438443
HelpText<"Add 'dynamic' to all declarations">;

lib/AST/Module.cpp

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2246,6 +2246,53 @@ SPIGroupsRequest::evaluate(Evaluator &evaluator, const Decl *decl) const {
22462246
return ctx.AllocateCopy(spiGroups.getArrayRef());
22472247
}
22482248

2249+
LibraryLevel ModuleDecl::getLibraryLevel() const {
2250+
return evaluateOrDefault(getASTContext().evaluator,
2251+
ModuleLibraryLevelRequest{this},
2252+
LibraryLevel::Other);
2253+
}
2254+
2255+
LibraryLevel
2256+
ModuleLibraryLevelRequest::evaluate(Evaluator &evaluator,
2257+
const ModuleDecl *module) const {
2258+
auto &ctx = module->getASTContext();
2259+
2260+
/// Is \p modulePath from System/Library/PrivateFrameworks/?
2261+
auto fromPrivateFrameworks = [&](StringRef modulePath) -> bool {
2262+
if (!ctx.LangOpts.Target.isOSDarwin()) return false;
2263+
2264+
namespace path = llvm::sys::path;
2265+
SmallString<128> scratch;
2266+
scratch = ctx.SearchPathOpts.SDKPath;
2267+
path::append(scratch, "System", "Library", "PrivateFrameworks");
2268+
return hasPrefix(path::begin(modulePath), path::end(modulePath),
2269+
path::begin(scratch), path::end(scratch));
2270+
};
2271+
2272+
if (module->isNonSwiftModule()) {
2273+
if (auto *underlying = module->findUnderlyingClangModule()) {
2274+
// Imported clangmodules are SPI if they are defined by a private
2275+
// modulemap or from the PrivateFrameworks folder in the SDK.
2276+
bool moduleIsSPI = underlying->ModuleMapIsPrivate ||
2277+
(underlying->isPartOfFramework() &&
2278+
fromPrivateFrameworks(underlying->PresumedModuleMapFile));
2279+
return moduleIsSPI ? LibraryLevel::SPI : LibraryLevel::API;
2280+
}
2281+
return LibraryLevel::Other;
2282+
2283+
} else if (module->isMainModule()) {
2284+
// The current compilation target.
2285+
return ctx.LangOpts.LibraryLevel;
2286+
2287+
} else {
2288+
// Other Swift modules are SPI if they are from the PrivateFrameworks
2289+
// folder in the SDK.
2290+
auto modulePath = module->getModuleFilename();
2291+
return fromPrivateFrameworks(modulePath) ?
2292+
LibraryLevel::SPI : LibraryLevel::API;
2293+
}
2294+
}
2295+
22492296
bool SourceFile::shouldCrossImport() const {
22502297
return Kind != SourceFileKind::SIL && Kind != SourceFileKind::Interface &&
22512298
getASTContext().LangOpts.EnableCrossImportOverlays;

lib/Frontend/CompilerInvocation.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,27 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
538538
Args.hasFlag(OPT_enable_swift3_objc_inference,
539539
OPT_disable_swift3_objc_inference, false);
540540

541+
if (const Arg *A = Args.getLastArg(OPT_library_level)) {
542+
StringRef contents = A->getValue();
543+
if (contents == "api") {
544+
Opts.LibraryLevel = LibraryLevel::API;
545+
} else if (contents == "spi") {
546+
Opts.LibraryLevel = LibraryLevel::SPI;
547+
} else {
548+
Opts.LibraryLevel = LibraryLevel::Other;
549+
if (contents != "other") {
550+
// Error on unknown library levels.
551+
auto inFlight = Diags.diagnose(SourceLoc(),
552+
diag::error_unknown_library_level,
553+
contents);
554+
555+
// Only warn for "ipi" as we may use it in the future.
556+
if (contents == "ipi")
557+
inFlight.limitBehavior(DiagnosticBehavior::Warning);
558+
}
559+
}
560+
}
561+
541562
if (Opts.EnableSwift3ObjCInference) {
542563
if (const Arg *A = Args.getLastArg(
543564
OPT_warn_swift3_objc_inference_minimal,

lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1610,6 +1610,25 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
16101610
// Force the lookup of decls referenced by a scoped import in case it emits
16111611
// diagnostics.
16121612
(void)ID->getDecls();
1613+
1614+
// Report the public import of a private module.
1615+
if (ID->getASTContext().LangOpts.LibraryLevel == LibraryLevel::API) {
1616+
auto target = ID->getModule();
1617+
auto importer = ID->getModuleContext();
1618+
if (target &&
1619+
!ID->getAttrs().hasAttribute<ImplementationOnlyAttr>() &&
1620+
target->getLibraryLevel() == LibraryLevel::SPI) {
1621+
1622+
auto &diags = ID->getASTContext().Diags;
1623+
InFlightDiagnostic inFlight =
1624+
diags.diagnose(ID, diag::warn_public_import_of_private_module,
1625+
target->getName(), importer->getName());
1626+
if (ID->getAttrs().isEmpty()) {
1627+
inFlight.fixItInsert(ID->getStartLoc(),
1628+
"@_implementationOnly ");
1629+
}
1630+
}
1631+
}
16131632
}
16141633

16151634
void visitOperatorDecl(OperatorDecl *OD) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
void bar() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
framework module PublicClang {
2+
umbrella header "PublicClang.h"
3+
export *
4+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
framework module PublicClang_Private {
2+
umbrella header "PublicClang_Private.h"
3+
export *
4+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
void bar() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// swift-interface-format-version: 1.0
2+
// swift-module-flags: -module-name PublicSwift -enable-library-evolution
3+
import Swift
4+
5+
public func foo() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
void bar() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
framework module FullyPrivateClang {
2+
umbrella header "FullyPrivateClang.h"
3+
4+
export *
5+
module * { export * }
6+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// swift-interface-format-version: 1.0
2+
// swift-module-flags: -module-name PrivateSwift
3+
import Swift
4+
5+
public func foo() {}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// RUN: %empty-directory(%t)
2+
// REQUIRES: VENDOR=apple
3+
4+
/// Expect warnings when building a public client.
5+
// RUN: %target-swift-frontend -sdk %S/Inputs/public-private-sdk -typecheck -module-cache-path %t %s \
6+
// RUN: -F %S/Inputs/public-private-sdk/System/Library/PrivateFrameworks/ \
7+
// RUN: -library-level api -verify -D PUBLIC_IMPORTS
8+
9+
/// Expect no warnings when building an SPI client.
10+
// RUN: %target-swift-frontend -sdk %S/Inputs/public-private-sdk -typecheck -module-cache-path %t %s \
11+
// RUN: -F %S/Inputs/public-private-sdk/System/Library/PrivateFrameworks/ \
12+
// RUN: -library-level spi -D PUBLIC_IMPORTS
13+
14+
/// Expect no warnings when building a client with some other library level.
15+
// RUN: %target-swift-frontend -sdk %S/Inputs/public-private-sdk -typecheck -module-cache-path %t %s \
16+
// RUN: -F %S/Inputs/public-private-sdk/System/Library/PrivateFrameworks/ \
17+
// RUN: -D PUBLIC_IMPORTS
18+
// RUN: %target-swift-frontend -sdk %S/Inputs/public-private-sdk -typecheck -module-cache-path %t %s \
19+
// RUN: -F %S/Inputs/public-private-sdk/System/Library/PrivateFrameworks/ \
20+
// RUN: -library-level other -D PUBLIC_IMPORTS
21+
#if PUBLIC_IMPORTS
22+
import PublicSwift
23+
import PrivateSwift // expected-warning{{private module 'PrivateSwift' is imported publicly from the public module 'main'}}
24+
25+
import PublicClang
26+
import PublicClang_Private // expected-warning{{private module 'PublicClang_Private' is imported publicly from the public module 'main'}}
27+
import FullyPrivateClang // expected-warning{{private module 'FullyPrivateClang' is imported publicly from the public module 'main'}}
28+
import main // expected-warning{{'implementation-only-import-suggestion.swift' is part of module 'main'; ignoring import}}
29+
30+
/// Expect no warnings with implementation-only imports.
31+
// RUN: %target-swift-frontend -sdk %S/Inputs/public-private-sdk -typecheck -module-cache-path %t %s \
32+
// RUN: -F %S/Inputs/public-private-sdk/System/Library/PrivateFrameworks/ \
33+
// RUN: -library-level api -D IMPL_ONLY_IMPORTS
34+
#elseif IMPL_ONLY_IMPORTS
35+
36+
@_implementationOnly import PrivateSwift
37+
@_implementationOnly import PublicClang_Private
38+
@_implementationOnly import FullyPrivateClang
39+
40+
#endif
41+
42+
/// Test error message on an unknown library level name.
43+
// RUN: not %target-swift-frontend -typecheck %s -library-level ThatsNotALibraryLevel 2>&1 \
44+
// RUN: | %FileCheck %s --check-prefix CHECK-ARG
45+
// CHECK-ARG: error: unknown library level 'ThatsNotALibraryLevel', expected one of 'api', 'spi' or 'other'

0 commit comments

Comments
 (0)