From be38abad01e2e6055c228024da85de30b54c1b1f Mon Sep 17 00:00:00 2001 From: Ellie Shin Date: Mon, 30 Oct 2023 13:38:16 -0700 Subject: [PATCH] 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 --- include/swift/AST/Decl.h | 19 ---------- include/swift/AST/Module.h | 2 ++ include/swift/Option/Options.td | 2 +- lib/AST/Decl.cpp | 36 ++++--------------- lib/Sema/TypeCheckProtocol.cpp | 4 --- test/ModuleInterface/lazy-typecheck.swift | 2 +- ...cessibility_package_inline_interface.swift | 2 +- .../accessibility_package_interface.swift | 6 ++-- test/Serialization/load_package_module.swift | 4 +-- test/Serialization/module_package_name.swift | 4 +-- 10 files changed, 19 insertions(+), 62 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index f2f5574f93306..80330b1554879 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -2690,25 +2690,6 @@ class ValueDecl : public Decl { /// \c \@usableFromInline, \c \@inlinalbe, and \c \@_alwaysEmitIntoClient bool isUsableFromInline() const; - /// Returns \c true if this value decl needs a special case handling for an - /// interface file. - /// - /// One such case is a reference of an inlinable decl with a `package` access level - /// in an interface file as follows: Package decls are only printed in interface files if - /// they are inlinable (as defined in \c isUsableFromInline). They could be - /// referenced by a module outside of its defining module that belong to the same - /// package determined by the `package-name` flag. However, the flag is only in - /// .swiftmodule and .private.swiftinterface, thus type checking references of inlinable - /// package symbols in public interfaces fails due to the missing flag. - /// Instead of adding the package-name flag to the public interfaces, which - /// could raise a security concern, we grant access to such cases. - /// - /// \sa useDC The use site where this value decl is referenced. - /// \sa useAcl The access level of its use site. - /// \sa declScope The access scope of this decl site. - bool skipAccessCheckIfInterface(const DeclContext *useDC, AccessLevel useAcl, - AccessScope declScope) const; - /// Returns \c true if this declaration is *not* intended to be used directly /// by application developers despite the visibility. bool shouldHideFromEditor() const; diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h index 19af4cf8df44d..4b599d515f544 100644 --- a/include/swift/AST/Module.h +++ b/include/swift/AST/Module.h @@ -209,6 +209,8 @@ class PackageUnit: public DeclContext { /// Equality check via package name instead of pointer comparison. /// Returns false if the name is empty. bool isSamePackageAs(PackageUnit *other) { + if (!other) + return false; return !(getName().empty()) && getName() == other->getName(); } }; diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td index 2b3fe7b5c4897..0fe3d042681b2 100644 --- a/include/swift/Option/Options.td +++ b/include/swift/Option/Options.td @@ -542,7 +542,7 @@ def module_abi_name : Separate<["-"], "module-abi-name">, Flags<[FrontendOption, ModuleInterfaceOption]>, HelpText<"ABI name to use for the contents of this module">; def package_name : Separate<["-"], "package-name">, - Flags<[FrontendOption, ModuleInterfaceOptionIgnorablePrivate]>, + Flags<[FrontendOption, ModuleInterfaceOption]>, HelpText<"Name of the package the module belongs to">; def export_as : Separate<["-"], "export-as">, Flags<[FrontendOption, ModuleInterfaceOption]>, diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index b735f6f22ec74..8fd0134e9dc2e 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -3826,17 +3826,6 @@ bool ValueDecl::isUsableFromInline() const { return false; } -bool ValueDecl::skipAccessCheckIfInterface(const DeclContext *useDC, - AccessLevel useAcl, - AccessScope declScope) const { - if (!useDC || useAcl != AccessLevel::Package || !declScope.isPackage() || - !isUsableFromInline() || - getDeclContext()->getParentModule() == useDC->getParentModule()) - return false; - auto useSF = useDC->getParentSourceFile(); - return useSF && useSF->Kind == SourceFileKind::Interface; -} - bool ValueDecl::shouldHideFromEditor() const { // Hide private stdlib declarations. if (isPrivateStdlibDecl(/*treatNonBuiltinProtocolsAsPublic*/ false) || @@ -4172,19 +4161,16 @@ static bool checkAccessUsingAccessScopes(const DeclContext *useDC, AccessScope accessScope = getAccessScopeForFormalAccess( VD, access, useDC, /*treatUsableFromInlineAsPublic*/ includeInlineable); - if (accessScope.getDeclContext() == useDC) return true; - if (!AccessScope(useDC).isChildOf(accessScope)) { - // Grant access if this VD is an inlinable package decl referenced by - // another module in an interface file. - if (VD->skipAccessCheckIfInterface(useDC, access, accessScope)) - return true; + if (accessScope.getDeclContext() == useDC) + return true; + if (!AccessScope(useDC).isChildOf(accessScope)) return false; - } // useDC is null only when caller wants to skip non-public type checks. - if (!useDC) return true; - + if (!useDC) + return true; // Check SPI access - if (!VD->isSPI()) return true; + if (!VD->isSPI()) + return true; auto useSF = dyn_cast(useDC->getModuleScopeContext()); return !useSF || useSF->isImportedAsSPI(VD) || VD->getDeclContext()->getParentModule() == useDC->getParentModule(); @@ -4304,14 +4290,6 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD, return useSF && useSF->hasTestableOrPrivateImport(access, sourceModule); } case AccessLevel::Package: { - auto srcFile = sourceDC->getParentSourceFile(); - - // srcFile could be null if VD decl is from an imported .swiftmodule - if (srcFile && srcFile->Kind == SourceFileKind::Interface) { - // If source file is interface, package decls must be usableFromInline or - // inlinable, and are accessed only within the defining module so return true - return true; - } auto srcPkg = sourceDC->getPackageContext(/*lookupIfNotCurrent*/ true); auto usePkg = useDC->getPackageContext(/*lookupIfNotCurrent*/ true); return srcPkg && usePkg && usePkg->isSamePackageAs(srcPkg); diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 9e5ead15d52d4..839a7be26fddf 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -4372,10 +4372,6 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) { requiredAccessScope.requiredAccessForDiagnostics(); auto proto = conformance->getProtocol(); auto protoAccessScope = proto->getFormalAccessScope(DC); - // Skip diagnostics of a witness of a package protocol that is inlinalbe - // referenced in an interface file. - if (proto->skipAccessCheckIfInterface(DC, requiredAccess, protoAccessScope)) - return; bool protoForcesAccess = requiredAccessScope.hasEqualDeclContextWith(protoAccessScope); auto diagKind = protoForcesAccess diff --git a/test/ModuleInterface/lazy-typecheck.swift b/test/ModuleInterface/lazy-typecheck.swift index 70d7de0044980..bbc051ec3e8f3 100644 --- a/test/ModuleInterface/lazy-typecheck.swift +++ b/test/ModuleInterface/lazy-typecheck.swift @@ -4,7 +4,7 @@ // RUN: %FileCheck %s < %t/lazy_typecheck.swiftinterface // RUN: rm -rf %t/*.swiftmodule -// RUN: %target-swift-frontend -package-name Package -typecheck -verify %S/../Inputs/lazy_typecheck_client.swift -I %t +// RUN: %target-swift-frontend -package-name ClientPackage -typecheck -verify %S/../Inputs/lazy_typecheck_client.swift -I %t // CHECK: import Swift diff --git a/test/Sema/accessibility_package_inline_interface.swift b/test/Sema/accessibility_package_inline_interface.swift index af0e5131564f0..13be5d1fbf8d6 100644 --- a/test/Sema/accessibility_package_inline_interface.swift +++ b/test/Sema/accessibility_package_inline_interface.swift @@ -9,8 +9,8 @@ // RUN: %target-swift-typecheck-module-from-interface(%t/Utils.swiftinterface) -I%t // RUN: %FileCheck %s -check-prefix CHECK-UTILS < %t/Utils.swiftinterface -// CHECK-UTILS-NOT: -package-name myLib // CHECK-UTILS: -module-name Utils +// CHECK-UTILS: -package-name myLib // CHECK-UTILS: @usableFromInline // CHECK-UTILS: package class PackageKlassProto { // CHECK-UTILS: @usableFromInline diff --git a/test/Sema/accessibility_package_interface.swift b/test/Sema/accessibility_package_interface.swift index c4679f5753c1e..c7302578154e1 100644 --- a/test/Sema/accessibility_package_interface.swift +++ b/test/Sema/accessibility_package_interface.swift @@ -12,13 +12,13 @@ // RUN: %target-swift-typecheck-module-from-interface(%t/Utils.swiftinterface) -I %t // RUN: %FileCheck %s --check-prefix=CHECK-PUBLIC-UTILS < %t/Utils.swiftinterface -// CHECK-PUBLIC-UTILS-NOT: -package-name swift-utils.log // CHECK-PUBLIC-UTILS-NOT: package func packageFunc() // CHECK-PUBLIC-UTILS-NOT: package protocol PackageProto // CHECK-PUBLIC-UTILS-NOT: var pkgVar // CHECK-PUBLIC-UTILS-NOT: package class PackageKlass // CHECK-PUBLIC-UTILS-NOT: package var pkgVar // CHECK-PUBLIC-UTILS: -module-name Utils +// CHECK-PUBLIC-UTILS: -package-name swift-utils.log // CHECK-PUBLIC-UTILS: public func publicFunc() // CHECK-PUBLIC-UTILS: @usableFromInline // CHECK-PUBLIC-UTILS: package func ufiPackageFunc() @@ -39,7 +39,7 @@ // CHECK-PRIVATE-UTILS-NOT: package class PackageKlass // CHECK-PRIVATE-UTILS-NOT: package var pkgVar // CHECK-PRIVATE-UTILS: -module-name Utils -// CHECK-PRIVATE-UTILS: swift-module-flags-ignorable-private: -package-name swift-utils.log +// CHECK-PRIVATE-UTILS: -package-name swift-utils.log // CHECK-PRIVATE-UTILS: public func publicFunc() // CHECK-PRIVATE-UTILS: @usableFromInline // CHECK-PRIVATE-UTILS: package func ufiPackageFunc() @@ -64,7 +64,7 @@ // RUN: %target-swift-typecheck-module-from-interface(%t/Client.swiftinterface) -I %t -verify // RUN: %FileCheck %s --check-prefix=CHECK-PUBLIC-CLIENT < %t/Client.swiftinterface -// CHECK-PUBLIC-CLIENT-NOT: -package-name swift-utils.log +// CHECK-PUBLIC-CLIENT: -package-name swift-utils.log // CHECK-PUBLIC-CLIENT: @inlinable public func clientFunc() // CHECK-PUBLIC-CLIENT: publicFunc() // CHECK-PUBLIC-CLIENT: ufiPackageFunc() diff --git a/test/Serialization/load_package_module.swift b/test/Serialization/load_package_module.swift index 36d30537e0206..9cb0fc0487819 100644 --- a/test/Serialization/load_package_module.swift +++ b/test/Serialization/load_package_module.swift @@ -19,11 +19,11 @@ // RUN: %target-swift-typecheck-module-from-interface(%t/LibInterface.swiftinterface) -I %t // RUN: %FileCheck %s --check-prefix=CHECK-PUBLIC < %t/LibInterface.swiftinterface // CHECK-PUBLIC: -module-name LibInterface -// CHECK-PUBLIC-NOT: -package-name +// CHECK-PUBLIC: -package-name // RUN: %target-swift-typecheck-module-from-interface(%t/LibInterface.private.swiftinterface) -module-name LibInterface -I %t // RUN: %FileCheck %s --check-prefix=CHECK-PRIVATE < %t/LibInterface.private.swiftinterface -// CHECK-PRIVATE: swift-module-flags-ignorable-private: -package-name libPkg +// CHECK-PRIVATE: -package-name libPkg // RUN: not %target-swift-frontend -typecheck %t/ClientLoadInterface.swift -package-name otherPkg -I %t 2> %t/resultX.output // RUN: %FileCheck %s -check-prefix CHECK-X < %t/resultX.output diff --git a/test/Serialization/module_package_name.swift b/test/Serialization/module_package_name.swift index c2b6ead0c4323..6bf48f7441544 100644 --- a/test/Serialization/module_package_name.swift +++ b/test/Serialization/module_package_name.swift @@ -11,12 +11,12 @@ // RUN: %target-swift-typecheck-module-from-interface(%t/Logging.swiftinterface) -I %t // RUN: %FileCheck %s --check-prefix=CHECK-PUBLIC < %t/Logging.swiftinterface // CHECK-PUBLIC: -module-name Logging -// CHECK-PUBLIC-NOT: -package-name +// CHECK-PUBLIC: -package-name // RUN: %target-swift-typecheck-module-from-interface(%t/Logging.private.swiftinterface) -module-name Logging -I %t // RUN: %FileCheck %s --check-prefix=CHECK-PRIVATE < %t/Logging.private.swiftinterface // CHECK-PRIVATE: -module-name Logging -// CHECK-PRIVATE: swift-module-flags-ignorable-private: -package-name MyLoggingPkg +// CHECK-PRIVATE: -package-name MyLoggingPkg //--- File.swift public func log(level: Int) {}