Skip to content

Commit be38aba

Browse files
committed
Package name is only printed in private swiftinterface. This causes
ambiguity during lookup when there are multiple public or inlinalbe package decls in public interfaces. This PR adds a package name to public swiftinterface and lets typecheck look up the package name to narrow down the scope of access to package decls from an external module. Resolves rdar://117699160
1 parent f93de71 commit be38aba

File tree

10 files changed

+19
-62
lines changed

10 files changed

+19
-62
lines changed

include/swift/AST/Decl.h

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2690,25 +2690,6 @@ class ValueDecl : public Decl {
26902690
/// \c \@usableFromInline, \c \@inlinalbe, and \c \@_alwaysEmitIntoClient
26912691
bool isUsableFromInline() const;
26922692

2693-
/// Returns \c true if this value decl needs a special case handling for an
2694-
/// interface file.
2695-
///
2696-
/// One such case is a reference of an inlinable decl with a `package` access level
2697-
/// in an interface file as follows: Package decls are only printed in interface files if
2698-
/// they are inlinable (as defined in \c isUsableFromInline). They could be
2699-
/// referenced by a module outside of its defining module that belong to the same
2700-
/// package determined by the `package-name` flag. However, the flag is only in
2701-
/// .swiftmodule and .private.swiftinterface, thus type checking references of inlinable
2702-
/// package symbols in public interfaces fails due to the missing flag.
2703-
/// Instead of adding the package-name flag to the public interfaces, which
2704-
/// could raise a security concern, we grant access to such cases.
2705-
///
2706-
/// \sa useDC The use site where this value decl is referenced.
2707-
/// \sa useAcl The access level of its use site.
2708-
/// \sa declScope The access scope of this decl site.
2709-
bool skipAccessCheckIfInterface(const DeclContext *useDC, AccessLevel useAcl,
2710-
AccessScope declScope) const;
2711-
27122693
/// Returns \c true if this declaration is *not* intended to be used directly
27132694
/// by application developers despite the visibility.
27142695
bool shouldHideFromEditor() const;

include/swift/AST/Module.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,8 @@ class PackageUnit: public DeclContext {
209209
/// Equality check via package name instead of pointer comparison.
210210
/// Returns false if the name is empty.
211211
bool isSamePackageAs(PackageUnit *other) {
212+
if (!other)
213+
return false;
212214
return !(getName().empty()) && getName() == other->getName();
213215
}
214216
};

include/swift/Option/Options.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ def module_abi_name : Separate<["-"], "module-abi-name">,
542542
Flags<[FrontendOption, ModuleInterfaceOption]>,
543543
HelpText<"ABI name to use for the contents of this module">;
544544
def package_name : Separate<["-"], "package-name">,
545-
Flags<[FrontendOption, ModuleInterfaceOptionIgnorablePrivate]>,
545+
Flags<[FrontendOption, ModuleInterfaceOption]>,
546546
HelpText<"Name of the package the module belongs to">;
547547
def export_as : Separate<["-"], "export-as">,
548548
Flags<[FrontendOption, ModuleInterfaceOption]>,

lib/AST/Decl.cpp

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3826,17 +3826,6 @@ bool ValueDecl::isUsableFromInline() const {
38263826
return false;
38273827
}
38283828

3829-
bool ValueDecl::skipAccessCheckIfInterface(const DeclContext *useDC,
3830-
AccessLevel useAcl,
3831-
AccessScope declScope) const {
3832-
if (!useDC || useAcl != AccessLevel::Package || !declScope.isPackage() ||
3833-
!isUsableFromInline() ||
3834-
getDeclContext()->getParentModule() == useDC->getParentModule())
3835-
return false;
3836-
auto useSF = useDC->getParentSourceFile();
3837-
return useSF && useSF->Kind == SourceFileKind::Interface;
3838-
}
3839-
38403829
bool ValueDecl::shouldHideFromEditor() const {
38413830
// Hide private stdlib declarations.
38423831
if (isPrivateStdlibDecl(/*treatNonBuiltinProtocolsAsPublic*/ false) ||
@@ -4172,19 +4161,16 @@ static bool checkAccessUsingAccessScopes(const DeclContext *useDC,
41724161
AccessScope accessScope = getAccessScopeForFormalAccess(
41734162
VD, access, useDC,
41744163
/*treatUsableFromInlineAsPublic*/ includeInlineable);
4175-
if (accessScope.getDeclContext() == useDC) return true;
4176-
if (!AccessScope(useDC).isChildOf(accessScope)) {
4177-
// Grant access if this VD is an inlinable package decl referenced by
4178-
// another module in an interface file.
4179-
if (VD->skipAccessCheckIfInterface(useDC, access, accessScope))
4180-
return true;
4164+
if (accessScope.getDeclContext() == useDC)
4165+
return true;
4166+
if (!AccessScope(useDC).isChildOf(accessScope))
41814167
return false;
4182-
}
41834168
// useDC is null only when caller wants to skip non-public type checks.
4184-
if (!useDC) return true;
4185-
4169+
if (!useDC)
4170+
return true;
41864171
// Check SPI access
4187-
if (!VD->isSPI()) return true;
4172+
if (!VD->isSPI())
4173+
return true;
41884174
auto useSF = dyn_cast<SourceFile>(useDC->getModuleScopeContext());
41894175
return !useSF || useSF->isImportedAsSPI(VD) ||
41904176
VD->getDeclContext()->getParentModule() == useDC->getParentModule();
@@ -4304,14 +4290,6 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
43044290
return useSF && useSF->hasTestableOrPrivateImport(access, sourceModule);
43054291
}
43064292
case AccessLevel::Package: {
4307-
auto srcFile = sourceDC->getParentSourceFile();
4308-
4309-
// srcFile could be null if VD decl is from an imported .swiftmodule
4310-
if (srcFile && srcFile->Kind == SourceFileKind::Interface) {
4311-
// If source file is interface, package decls must be usableFromInline or
4312-
// inlinable, and are accessed only within the defining module so return true
4313-
return true;
4314-
}
43154293
auto srcPkg = sourceDC->getPackageContext(/*lookupIfNotCurrent*/ true);
43164294
auto usePkg = useDC->getPackageContext(/*lookupIfNotCurrent*/ true);
43174295
return srcPkg && usePkg && usePkg->isSamePackageAs(srcPkg);

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4372,10 +4372,6 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
43724372
requiredAccessScope.requiredAccessForDiagnostics();
43734373
auto proto = conformance->getProtocol();
43744374
auto protoAccessScope = proto->getFormalAccessScope(DC);
4375-
// Skip diagnostics of a witness of a package protocol that is inlinalbe
4376-
// referenced in an interface file.
4377-
if (proto->skipAccessCheckIfInterface(DC, requiredAccess, protoAccessScope))
4378-
return;
43794375
bool protoForcesAccess =
43804376
requiredAccessScope.hasEqualDeclContextWith(protoAccessScope);
43814377
auto diagKind = protoForcesAccess

test/ModuleInterface/lazy-typecheck.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
// RUN: %FileCheck %s < %t/lazy_typecheck.swiftinterface
55

66
// RUN: rm -rf %t/*.swiftmodule
7-
// RUN: %target-swift-frontend -package-name Package -typecheck -verify %S/../Inputs/lazy_typecheck_client.swift -I %t
7+
// RUN: %target-swift-frontend -package-name ClientPackage -typecheck -verify %S/../Inputs/lazy_typecheck_client.swift -I %t
88

99
// CHECK: import Swift
1010

test/Sema/accessibility_package_inline_interface.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
// RUN: %target-swift-typecheck-module-from-interface(%t/Utils.swiftinterface) -I%t
1010

1111
// RUN: %FileCheck %s -check-prefix CHECK-UTILS < %t/Utils.swiftinterface
12-
// CHECK-UTILS-NOT: -package-name myLib
1312
// CHECK-UTILS: -module-name Utils
13+
// CHECK-UTILS: -package-name myLib
1414
// CHECK-UTILS: @usableFromInline
1515
// CHECK-UTILS: package class PackageKlassProto {
1616
// CHECK-UTILS: @usableFromInline

test/Sema/accessibility_package_interface.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@
1212
// RUN: %target-swift-typecheck-module-from-interface(%t/Utils.swiftinterface) -I %t
1313
// RUN: %FileCheck %s --check-prefix=CHECK-PUBLIC-UTILS < %t/Utils.swiftinterface
1414

15-
// CHECK-PUBLIC-UTILS-NOT: -package-name swift-utils.log
1615
// CHECK-PUBLIC-UTILS-NOT: package func packageFunc()
1716
// CHECK-PUBLIC-UTILS-NOT: package protocol PackageProto
1817
// CHECK-PUBLIC-UTILS-NOT: var pkgVar
1918
// CHECK-PUBLIC-UTILS-NOT: package class PackageKlass
2019
// CHECK-PUBLIC-UTILS-NOT: package var pkgVar
2120
// CHECK-PUBLIC-UTILS: -module-name Utils
21+
// CHECK-PUBLIC-UTILS: -package-name swift-utils.log
2222
// CHECK-PUBLIC-UTILS: public func publicFunc()
2323
// CHECK-PUBLIC-UTILS: @usableFromInline
2424
// CHECK-PUBLIC-UTILS: package func ufiPackageFunc()
@@ -39,7 +39,7 @@
3939
// CHECK-PRIVATE-UTILS-NOT: package class PackageKlass
4040
// CHECK-PRIVATE-UTILS-NOT: package var pkgVar
4141
// CHECK-PRIVATE-UTILS: -module-name Utils
42-
// CHECK-PRIVATE-UTILS: swift-module-flags-ignorable-private: -package-name swift-utils.log
42+
// CHECK-PRIVATE-UTILS: -package-name swift-utils.log
4343
// CHECK-PRIVATE-UTILS: public func publicFunc()
4444
// CHECK-PRIVATE-UTILS: @usableFromInline
4545
// CHECK-PRIVATE-UTILS: package func ufiPackageFunc()
@@ -64,7 +64,7 @@
6464

6565
// RUN: %target-swift-typecheck-module-from-interface(%t/Client.swiftinterface) -I %t -verify
6666
// RUN: %FileCheck %s --check-prefix=CHECK-PUBLIC-CLIENT < %t/Client.swiftinterface
67-
// CHECK-PUBLIC-CLIENT-NOT: -package-name swift-utils.log
67+
// CHECK-PUBLIC-CLIENT: -package-name swift-utils.log
6868
// CHECK-PUBLIC-CLIENT: @inlinable public func clientFunc()
6969
// CHECK-PUBLIC-CLIENT: publicFunc()
7070
// CHECK-PUBLIC-CLIENT: ufiPackageFunc()

test/Serialization/load_package_module.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919
// RUN: %target-swift-typecheck-module-from-interface(%t/LibInterface.swiftinterface) -I %t
2020
// RUN: %FileCheck %s --check-prefix=CHECK-PUBLIC < %t/LibInterface.swiftinterface
2121
// CHECK-PUBLIC: -module-name LibInterface
22-
// CHECK-PUBLIC-NOT: -package-name
22+
// CHECK-PUBLIC: -package-name
2323

2424
// RUN: %target-swift-typecheck-module-from-interface(%t/LibInterface.private.swiftinterface) -module-name LibInterface -I %t
2525
// RUN: %FileCheck %s --check-prefix=CHECK-PRIVATE < %t/LibInterface.private.swiftinterface
26-
// CHECK-PRIVATE: swift-module-flags-ignorable-private: -package-name libPkg
26+
// CHECK-PRIVATE: -package-name libPkg
2727

2828
// RUN: not %target-swift-frontend -typecheck %t/ClientLoadInterface.swift -package-name otherPkg -I %t 2> %t/resultX.output
2929
// RUN: %FileCheck %s -check-prefix CHECK-X < %t/resultX.output

test/Serialization/module_package_name.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@
1111
// RUN: %target-swift-typecheck-module-from-interface(%t/Logging.swiftinterface) -I %t
1212
// RUN: %FileCheck %s --check-prefix=CHECK-PUBLIC < %t/Logging.swiftinterface
1313
// CHECK-PUBLIC: -module-name Logging
14-
// CHECK-PUBLIC-NOT: -package-name
14+
// CHECK-PUBLIC: -package-name
1515

1616
// RUN: %target-swift-typecheck-module-from-interface(%t/Logging.private.swiftinterface) -module-name Logging -I %t
1717
// RUN: %FileCheck %s --check-prefix=CHECK-PRIVATE < %t/Logging.private.swiftinterface
1818
// CHECK-PRIVATE: -module-name Logging
19-
// CHECK-PRIVATE: swift-module-flags-ignorable-private: -package-name MyLoggingPkg
19+
// CHECK-PRIVATE: -package-name MyLoggingPkg
2020

2121
//--- File.swift
2222
public func log(level: Int) {}

0 commit comments

Comments
 (0)