Skip to content

Commit cf58bb7

Browse files
committed
[Sema] Report public imports of private modules
Intro the concept of library access or distribution level to identify layers of libraries and report public imports of private libraries from public ones. rdar://62934005
1 parent 58f03e8 commit cf58bb7

File tree

19 files changed

+207
-0
lines changed

19 files changed

+207
-0
lines changed

Diff for: include/swift/AST/DiagnosticsFrontend.def

+4
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

Diff for: include/swift/AST/DiagnosticsSema.def

+3
Original file line numberDiff line numberDiff line change
@@ -2834,6 +2834,9 @@ WARNING(warn_implementation_only_conflict,none,
28342834
(Identifier))
28352835
NOTE(implementation_only_conflict_here,none,
28362836
"imported as implementation-only here", ())
2837+
WARNING(warn_public_import_of_private_module,none,
2838+
"private module %0 is imported publicly from the public module %1",
2839+
(Identifier, Identifier))
28372840

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

Diff for: include/swift/AST/Module.h

+4
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) {

Diff for: include/swift/AST/TypeCheckRequests.h

+18
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 *,

Diff for: include/swift/AST/TypeCheckerTypeIDZone.def

+2
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,

Diff for: include/swift/Basic/LangOptions.h

+17
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 =

Diff for: include/swift/Option/FrontendOptions.td

+5
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">;

Diff for: lib/AST/Module.cpp

+43
Original file line numberDiff line numberDiff line change
@@ -2246,6 +2246,49 @@ 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 path from System/Library/PrivateFrameworks/?
2261+
auto fromPrivateFrameworks = [&](StringRef path) -> bool {
2262+
auto sep = llvm::sys::path::get_separator();
2263+
auto privateFrameworksPath = llvm::Twine(ctx.SearchPathOpts.SDKPath) +
2264+
sep + "System" + sep + "Library" + sep + "PrivateFrameworks" + sep;
2265+
return hasPrefix(path, privateFrameworksPath.str());
2266+
};
2267+
2268+
if (module->isNonSwiftModule()) {
2269+
if (auto *underlying = module->findUnderlyingClangModule()) {
2270+
// Imported clangmodules are SPI if they are defined by a private
2271+
// modulemap or from the PrivateFrameworks folder in the SDK.
2272+
bool moduleIsSPI = underlying->ModuleMapIsPrivate ||
2273+
(underlying->isPartOfFramework() &&
2274+
fromPrivateFrameworks(underlying->PresumedModuleMapFile));
2275+
return moduleIsSPI ? LibraryLevel::SPI : LibraryLevel::API;
2276+
}
2277+
return LibraryLevel::Other;
2278+
2279+
} else if (module->isMainModule()) {
2280+
// The current compilation target.
2281+
return ctx.LangOpts.LibraryLevel;
2282+
2283+
} else {
2284+
// Other Swift modules are SPI if they are from the PrivateFrameworks
2285+
// folder in the SDK.
2286+
auto modulePath = module->getModuleFilename();
2287+
return fromPrivateFrameworks(modulePath) ?
2288+
LibraryLevel::SPI : LibraryLevel::API;
2289+
}
2290+
}
2291+
22492292
bool SourceFile::shouldCrossImport() const {
22502293
return Kind != SourceFileKind::SIL && Kind != SourceFileKind::Interface &&
22512294
getASTContext().LangOpts.EnableCrossImportOverlays;

Diff for: lib/Frontend/CompilerInvocation.cpp

+21
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,

Diff for: lib/Sema/TypeCheckDeclPrimary.cpp

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

0 commit comments

Comments
 (0)