Merge pull request #69545 from apple/es-pkg-public

elsh · web-flow · commit 3de51e34d01f · 2023-10-31T17:40:22.000-07:00
[5.10] Add package name to public swiftinterface
diff --git 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
@@ -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
@@ -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
@@ -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<SourceFile>(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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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) {}

Original file line number	Diff line number	Diff line change
`@@ -209,6 +209,8 @@ class PackageUnit: public DeclContext {`
`209`	`209`	`/// Equality check via package name instead of pointer comparison.`
`210`	`210`	`/// Returns false if the name is empty.`
`211`	`211`	`bool isSamePackageAs(PackageUnit *other) {`
	`212`	`+ if (!other)`
	`213`	`+ return false;`
`212`	`214`	`return !(getName().empty()) && getName() == other->getName();`
`213`	`215`	`}`
`214`	`216`	`};`