Merge pull request #77408 from tshortli/revert-package-fixes

tshortli · web-flow · commit b345418eca2d · 2024-11-06T10:31:34.000-08:00
Revert removal of `-package-name` from swiftinterfaces
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -2913,14 +2913,6 @@ class ValueDecl : public Decl {
   /// \c \@usableFromInline, \c \@inlinalbe, and \c \@_alwaysEmitIntoClient
   bool isUsableFromInline() const;
 
-  /// Treat as public and allow skipping access checks if the following conditions
-  /// are met:
-  /// - This decl has a package access level,
-  /// - Has a @usableFromInline (or other inlinable) attribute,
-  /// - And is defined in a module built from a public or private
-  ///   interface that does not contain package-name.
-  bool isInterfacePackageEffectivelyPublic() 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/Frontend/ModuleInterfaceSupport.h b/include/swift/Frontend/ModuleInterfaceSupport.h
@@ -72,6 +72,10 @@ struct ModuleInterfaceOptions {
   /// Print imports that are missing from the source and used in API.
   bool PrintMissingImports = true;
 
+  /// If true, package-name flag is not printed in either public or private
+  /// interface file.
+  bool DisablePackageNameForNonPackageInterface = false;
+
   /// Intentionally print invalid syntax into the file.
   bool DebugPrintInvalidSyntax = false;
 
diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td
@@ -760,12 +760,8 @@ def disable_bridging_pch : Flag<["-"], "disable-bridging-pch">,
 
 def disable_print_package_name_for_non_package_interface :
   Flag<["-"], "disable-print-package-name-for-non-package-interface">,
-  Flags<[FrontendOption, NoDriverOption, HelpHidden]>,
-  HelpText<"No op; package name is only printed in package interface by default">;
-def print_package_name_in_non_package_interface :
-  Flag<["-"], "print-package-name-in-non-package-interface">,
-  Flags<[FrontendOption, NoDriverOption, HelpHidden]>,
-  HelpText<"Print package name in public or private interface">;
+  Flags<[FrontendOption, NoDriverOption, ModuleInterfaceOption, HelpHidden]>,
+  HelpText<"Disable adding package name to public or private interface">;
 
 def lto : Joined<["-"], "lto=">,
   Flags<[FrontendOption, NoInteractiveOption]>,
diff --git a/lib/AST/ASTVerifier.cpp b/lib/AST/ASTVerifier.cpp
@@ -1016,8 +1016,7 @@ class Verifier : public ASTWalker {
         PrettyStackTraceDecl debugStack("verifying access", D);
         if (!D->getASTContext().isAccessControlDisabled()) {
           if (D->getFormalAccessScope().isPublic() &&
-              D->getFormalAccess() < AccessLevel::Public &&
-              !D->isInterfacePackageEffectivelyPublic()) {
+              D->getFormalAccess() < AccessLevel::Public) {
             Out << "non-public decl has no formal access scope\n";
             D->dump(Out);
             abort();
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -4344,30 +4344,6 @@ bool ValueDecl::isUsableFromInline() const {
   return false;
 }
 
-bool ValueDecl::isInterfacePackageEffectivelyPublic() const {
-  // A package decl with @usableFromInline (or other inlinable
-  // attributes) is essentially public, and can be printed in
-  // public (or private) interface file without package-name;
-  // it can be referenced by another module (without package-name)
-  // importing such interface module.
-  auto isCandidate = getFormalAccess() == AccessLevel::Package &&
-                     isUsableFromInline() &&
-                     getModuleContext()->getPackageName().empty();
-  if (!isCandidate)
-    return false;
-
-  // Treat the decl as public (1) if it's contained in an interface
-  // file, e.g. when running -typecheck-module-from-interface or
-  // -compile-module-from-interface.
-  isCandidate = false;
-  if (auto srcFile = getDeclContext()->getParentSourceFile()) {
-    isCandidate = srcFile->Kind == SourceFileKind::Interface;
-  }
-  // Or (2) if the decl being referenced in a client file is defined
-  // in an interface module.
-  return isCandidate || getModuleContext()->isBuiltFromInterface();
-}
-
 bool ValueDecl::shouldHideFromEditor() const {
   // Hide private stdlib declarations.
   if (isPrivateSystemDecl(/*treatNonBuiltinProtocolsAsPublic*/ false) ||
@@ -4460,9 +4436,6 @@ static AccessLevel getAdjustedFormalAccess(const ValueDecl *VD,
   if (useDC && VD->getASTContext().isAccessControlDisabled())
     return getMaximallyOpenAccessFor(VD);
 
-  if (VD->isInterfacePackageEffectivelyPublic())
-    return AccessLevel::Public;
-
   if (treatUsableFromInlineAsPublic &&
       access < AccessLevel::Public &&
       VD->isUsableFromInline()) {
@@ -4645,11 +4618,9 @@ getAccessScopeForFormalAccess(const ValueDecl *VD,
   case AccessLevel::Package: {
     auto pkg = resultDC->getPackageContext(/*lookupIfNotCurrent*/ true);
     if (!pkg) {
-      if (VD->isInterfacePackageEffectivelyPublic())
-        return AccessScope::getPublic();
-
-      // If reached here, should be treated as internal.
-      return AccessScope(resultDC->getParentModule());
+      // Instead of reporting and failing early, return the scope of resultDC to
+      // allow continuation (should still non-zero exit later if in script mode)
+      return AccessScope(resultDC);
     } else {
       return AccessScope(pkg);
     }
@@ -4782,9 +4753,6 @@ static bool checkAccess(const DeclContext *useDC, const ValueDecl *VD,
   if (VD->getASTContext().isAccessControlDisabled())
     return true;
 
-  if (VD->isInterfacePackageEffectivelyPublic())
-    return true;
-
   auto access = getAccessLevel();
   auto *sourceDC = VD->getDeclContext();
 
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
@@ -524,6 +524,7 @@ static void ParseModuleInterfaceArgs(ModuleInterfaceOptions &Opts,
   Opts.PrintMissingImports =
     !Args.hasArg(OPT_disable_print_missing_imports_in_module_interface);
   Opts.ABIComments = Args.hasArg(OPT_abi_comments_in_module_interface);
+  Opts.DisablePackageNameForNonPackageInterface |= Args.hasArg(OPT_disable_print_package_name_for_non_package_interface);
 
   if (const Arg *A = Args.getLastArg(OPT_library_level)) {
     StringRef contents = A->getValue();
@@ -555,8 +556,8 @@ static bool ShouldIncludeModuleInterfaceArg(const Arg *A) {
 
 static bool IsPackageInterfaceFlag(const Arg *A, ArgList &Args) {
   return A->getOption().matches(options::OPT_package_name) &&
-         !Args.hasArg(
-             options::OPT_print_package_name_in_non_package_interface);
+         Args.hasArg(
+             options::OPT_disable_print_package_name_for_non_package_interface);
 }
 
 static bool IsPrivateInterfaceFlag(const Arg *A, ArgList &Args) {
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -4312,9 +4312,6 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
           requiredAccessScope.requiredAccessForDiagnostics();
         auto proto = conformance->getProtocol();
         auto protoAccessScope = proto->getFormalAccessScope(DC);
-        if (proto->isInterfacePackageEffectivelyPublic())
-          return;
-
         bool protoForcesAccess =
           requiredAccessScope.hasEqualDeclContextWith(protoAccessScope);
         auto diagKind = protoForcesAccess
diff --git a/test/ModuleInterface/load_package_interface.swift b/test/ModuleInterface/load_package_interface.swift
@@ -5,7 +5,6 @@
 // RUN: %target-swift-frontend -emit-module %t/Bar.swift -I %t \
 // RUN:   -module-name Bar -package-name foopkg -package-name barpkg \
 // RUN:   -enable-library-evolution -swift-version 5 \
-// RUN:   -disable-print-package-name-for-non-package-interface \
 // RUN:   -emit-module-interface-path %t/Bar.swiftinterface \
 // RUN:   -emit-private-module-interface-path %t/Bar.private.swiftinterface \
 // RUN:   -emit-package-module-interface-path %t/Bar.package.swiftinterface
@@ -32,8 +31,7 @@
 // RUN:   -package-name barpkg \
 // RUN:   -Rmodule-loading 2> %t/load-pkg-off.output
 // RUN: %FileCheck -check-prefix=CHECK-LOAD-PKG-OFF %s < %t/load-pkg-off.output
-// CHECK-LOAD-PKG-OFF: remark: loaded module 'Bar'; source: '{{.*}}Bar.private.swiftinterface', loaded: '{{.*}}Bar-{{.*}}.swiftmodule'
-// CHECK-LOAD-PKG-OFF: error: cannot find 'PkgKlass' in scope; did you mean 'PubKlass'?
+// CHECK-LOAD-PKG-OFF: error: module 'Bar' is in package 'barpkg' but was built from a non-package interface; modules of the same package can only be loaded if built from source or package interface:{{.*}}Bar.private.swiftinterface
 
 /// Client loads a private interface since the package-name is different from the loaded module's.
 // RUN: not %target-swift-frontend -typecheck %t/Client.swift -I %t \
@@ -47,15 +45,13 @@
 // RUN: rm -rf %t/*.swiftmodule
 // RUN: rm -rf %t/Bar.package.swiftinterface
 
-/// Client loads a private interface since package interface doesn't exist. It should error since the
-/// loaded module is not built from a package interface.
+/// Client loads a private interface since package interface doesn't exist. It should error since the loaded module is not built from a package interface.
 // RUN: not %target-swift-frontend -typecheck %t/Client.swift -I %t \
 // RUN:   -package-name barpkg \
 // RUN:   -experimental-package-interface-load \
 // RUN:   -Rmodule-loading 2> %t/load-priv.output
 // RUN: %FileCheck -check-prefix=CHECK-LOAD-PRIV %s < %t/load-priv.output
-// CHECK-LOAD-PRIV: remark: loaded module 'Bar'; source: '{{.*}}Bar.private.swiftinterface', loaded: '{{.*}}Bar-{{.*}}.swiftmodule'
-// CHECK-LOAD-PRIV: error: cannot find 'PkgKlass' in scope; did you mean 'PubKlass'?
+// CHECK-LOAD-PRIV: no such module 'Bar'
 
 // RUN: rm -rf %t/*.swiftmodule
 // RUN: rm -rf %t/Bar.private.swiftinterface
@@ -68,8 +64,7 @@
 // RUN:   -Rmodule-loading 2> %t/load-pub.output
 
 // RUN: %FileCheck -check-prefix=CHECK-LOAD-PUB %s < %t/load-pub.output
-// CHECK-LOAD-PUB: remark: loaded module 'Bar'; source: '{{.*}}Bar.swiftinterface', loaded: '{{.*}}Bar-{{.*}}.swiftmodule'
-// CHECK-LOAD-PUB: error: cannot find 'PkgKlass' in scope; did you mean 'PubKlass'?
+// CHECK-LOAD-PUB: no such module 'Bar'
 
 //--- Bar.swift
 public class PubKlass {
diff --git a/test/ModuleInterface/package_interface.swift b/test/ModuleInterface/package_interface.swift
@@ -23,6 +23,7 @@ public enum PubEnum {
   case red, green
 }
 
+// CHECK: -package-name barpkg
 // CHECK: public enum PubEnum {
 // CHECK:   case red, green
 // CHECK:   public static func == (a: Bar.PubEnum, b: Bar.PubEnum) -> Swift.Bool
diff --git a/test/ModuleInterface/package_interface_disable_package_name.swift b/test/ModuleInterface/package_interface_disable_package_name.swift
@@ -1,11 +1,11 @@
 // RUN: %empty-directory(%t)
-// RUN: split-file %s %t
 
-/// By default, package-name is only printed in package interface
-// RUN: %target-build-swift -emit-module %t/Bar.swift -I %t \
+/// Do not print package-name for public or private interfaces
+// RUN: %target-build-swift -emit-module %s -I %t \
 // RUN:   -module-name Bar -package-name foopkg \
 // RUN:   -enable-library-evolution -swift-version 6 \
 // RUN:   -package-name barpkg \
+// RUN:   -Xfrontend -disable-print-package-name-for-non-package-interface \
 // RUN:   -emit-module-interface-path %t/Bar.swiftinterface \
 // RUN:   -emit-private-module-interface-path %t/Bar.private.swiftinterface \
 // RUN:   -emit-package-module-interface-path %t/Bar.package.swiftinterface
@@ -22,9 +22,9 @@
 // CHECK-PRIVATE-NOT: pkgVar
 // CHECK-PACKAGE-NOT: -package-name foopkg
 
-// CHECK-PUBLIC:  -enable-library-evolution -swift-version 6 -module-name Bar
-// CHECK-PRIVATE:  -enable-library-evolution -swift-version 6 -module-name Bar
-// CHECK-PACKAGE:  -enable-library-evolution -swift-version 6 -module-name Bar -package-name barpkg
+// CHECK-PUBLIC:  -enable-library-evolution -swift-version 6 -disable-print-package-name-for-non-package-interface -module-name Bar
+// CHECK-PRIVATE:  -enable-library-evolution -swift-version 6 -disable-print-package-name-for-non-package-interface -module-name Bar
+// CHECK-PACKAGE:  -enable-library-evolution -swift-version 6 -disable-print-package-name-for-non-package-interface -module-name Bar -package-name barpkg
 
 /// Typechecking interface files (without package-name in non-package interface) should succeed.
 // RUN: %target-swift-frontend -typecheck-module-from-interface %t/Bar.swiftinterface
@@ -33,31 +33,21 @@
 
 /// Verify building modules from non-package interfaces succeeds without the package-name flag.
 // RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
-// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -verify
-// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -verify
-
 // RUN: rm -rf %t/Bar.swiftmodule
 // RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.private.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
-// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -verify
-// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -verify
-
 // RUN: rm -rf %t/Bar.swiftmodule
 // RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.package.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
-// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -experimental-package-interface-load -verify
 
 // RUN: rm -rf %t/Bar.swiftmodule
 // RUN: rm -rf %t/Bar.swiftinterface
 // RUN: rm -rf %t/Bar.private.swiftinterface
 // RUN: rm -rf %t/Bar.package.swiftinterface
 
-/// Print -package-name in public or private interface.
-/// Note the order of arguments differs across old/new driver, so force old
-/// driver for now.
+/// By default, -package-name is printed in all interfaces.
 // RUN: env SWIFT_USE_OLD_DRIVER=1 %target-build-swift \
-// RUN:   -emit-module %t/Bar.swift -I %t \
+// RUN:   -emit-module %s -I %t \
 // RUN:   -module-name Bar -package-name barpkg \
 // RUN:   -enable-library-evolution -swift-version 6 \
-// RUN:   -Xfrontend -print-package-name-in-non-package-interface \
 // RUN:   -emit-module-interface-path %t/Bar.swiftinterface \
 // RUN:   -emit-private-module-interface-path %t/Bar.private.swiftinterface \
 // RUN:   -emit-package-module-interface-path %t/Bar.package.swiftinterface
@@ -70,51 +60,12 @@
 
 /// Building modules from non-package interfaces with package-name (default mode) should succeed.
 // RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
-// RUN: %target-swift-frontend -typecheck %t/ExpectFail.swift -I %t -package-name barpkg -verify
-
 // RUN: rm -rf %t/Bar.swiftmodule
 // RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.private.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
-// RUN: %target-swift-frontend -typecheck %t/ExpectFail.swift -I %t -package-name barpkg -verify
-
 // RUN: rm -rf %t/Bar.swiftmodule
 // RUN: %target-swift-frontend -compile-module-from-interface %t/Bar.package.swiftinterface -o %t/Bar.swiftmodule -module-name Bar
-// RUN: %target-swift-frontend -typecheck %t/Use.swift -I %t -package-name barpkg -experimental-package-interface-load -verify
-
-//--- Bar.swift
-public struct PubStruct {
-  public var pubVar: Int
-  package var pkgVar: Int
-}
 
+public struct PubStruct {}
 @_spi(bar) public struct SPIStruct {}
 
 package struct PkgStruct {}
-
-@usableFromInline
-package struct UfiPkgStruct {
-  @usableFromInline
-  package var ufiPkgVar: PubStruct
-  
-  package var pkgVar: PubStruct
-  
-  @usableFromInline
-  package init(_ x: PubStruct, _ y: PubStruct) {
-    ufiPkgVar = x
-    pkgVar = y
-  }
-}
-
-
-//--- Use.swift
-import Bar
-
-func use(_ arg: PubStruct) -> PubStruct {
-  return UfiPkgStruct(arg, arg).ufiPkgVar
-}
-
-//--- ExpectFail.swift
-import Bar // expected-error {{module 'Bar' is in package 'barpkg' but was built from a non-package interface; modules of the same package can only be loaded if built from source or package interface}}
-
-func use(_ arg: PubStruct) -> PubStruct {
-  return UfiPkgStruct(arg, arg).ufiPkgVar
-}
diff --git a/test/Sema/accessibility_package_import_interface.swift b/test/Sema/accessibility_package_import_interface.swift
@@ -6,14 +6,18 @@
 // RUN:   -module-name Dep -swift-version 5 -I %t \
 // RUN:   -package-name myPkg \
 // RUN:   -enable-library-evolution \
-// RUN:   -disable-print-package-name-for-non-package-interface \
 // RUN:   -emit-module-path %t/Dep.swiftmodule \
 // RUN:   -emit-module-interface-path %t/Dep.swiftinterface \
 // RUN:   -emit-private-module-interface-path %t/Dep.private.swiftinterface
 
+// TEST Dep private interface should contain the package name
+// RUN: %target-swift-typecheck-module-from-interface(%t/Dep.private.swiftinterface) -module-name Dep -I %t
+// RUN: %FileCheck %s --check-prefix=CHECK-DEP-PRIVATE < %t/Dep.private.swiftinterface
+// CHECK-DEP-PRIVATE: -package-name myPkg
+
 // TEST Dep.swiftmodule should contain package name and package symbols
 // RUN: llvm-bcanalyzer --dump %t/Dep.swiftmodule | %FileCheck %s --check-prefix=CHECK-DEP-BC
-// CHECK-DEP-BC: <MODULE_PACKAGE_NAME {{.*}}> blob data = 'myPkg'
+// CHECK-DEP-BC: <MODULE_PACKAGE_NAME abbrevid=6/> blob data = 'myPkg'
 
 // TEST Lib should load Dep.swiftmodule and access package decls if in the same package and error if not
 // RUN: %target-swift-frontend -typecheck %t/Lib.swift -package-name myPkg -I %t
@@ -35,9 +39,9 @@
 // RUN:   -module-name Dep -I %t \
 // RUN:   -o %t/Dep.swiftmodule
 
-// TEST Dep binary built from interface should not contain package name or package symbols.
+// TEST Dep binary built from interface should contain package name but no package symbols
 // RUN: llvm-bcanalyzer --dump %t/Dep.swiftmodule | %FileCheck %s --check-prefix=CHECK-DEP-INTER-BC
-// CHECK-DEP-INTER-BC-NOT: <MODULE_PACKAGE_NAME {{.*}}> blob data = 'myPkg'
+// CHECK-DEP-INTER-BC: <MODULE_PACKAGE_NAME abbrevid=7/> blob data = 'myPkg'
 
 // TEST Lib should error on loading Dep built from interface and accessing package symbols (unless usableFromInline or inlinable)
 // RUN: %target-swift-frontend -typecheck %t/Lib.swift -package-name myPkg -I %t -verify
@@ -55,7 +59,6 @@
 // RUN:   -module-name LibPass -swift-version 5 -I %t \
 // RUN:   -package-name myPkg \
 // RUN:   -enable-library-evolution \
-// RUN:   -disable-print-package-name-for-non-package-interface \
 // RUN:   -emit-module-path %t/LibPass.swiftmodule \
 // RUN:   -emit-module-interface-path %t/LibPass.swiftinterface \
 // RUN:   -emit-private-module-interface-path %t/LibPass.private.swiftinterface
@@ -83,11 +86,8 @@
 @usableFromInline
 package class PackageKlassUFI {
   @usableFromInline package init() {}
-  package var packageVar: String = "pkg"
-
   @usableFromInline package var packageVarUFI: String = "pkgUFI"
-  // expected-note@-1 * {{'packageVarUFI' declared here}}
-  // expected-error@-1 * {{'packageVarUFI' declared here}}
+  package var packageVar: String = "pkg"
 }
 
 package func packageFunc() {
@@ -109,7 +109,7 @@ public func publicFuncInlinable() {
 }
 
 //--- Lib.swift
-import Dep
+import Dep // expected-error {{module 'Dep' is in package 'myPkg' but was built from a non-package interface; modules of the same package can only be loaded if built from source or package interface}}
 
 public func libFunc() {
   publicFuncInlinable()
@@ -118,7 +118,8 @@ public func libFunc() {
   packageFunc() // expected-error {{cannot find 'packageFunc' in scope}}
   let x = PackageKlassUFI()
   let y = x.packageVarUFI
-  print(x, y)
+  let z = x.packageVar // expected-error {{value of type 'PackageKlassUFI' has no member 'packageVar'}}
+  print(x, y, z)
 }
 
 
diff --git a/test/Sema/accessibility_package_inline_interface.swift b/test/Sema/accessibility_package_inline_interface.swift
diff --git a/test/Sema/accessibility_package_interface.swift b/test/Sema/accessibility_package_interface.swift
diff --git a/test/Sema/package_enums_treat_exhaustive.swift b/test/Sema/package_enums_treat_exhaustive.swift
diff --git a/test/Serialization/load_package_module.swift b/test/Serialization/load_package_module.swift
diff --git a/test/Serialization/module_package_name.swift b/test/Serialization/module_package_name.swift

Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@ public enum PubEnum {`
`23`	`23`	`case red, green`
`24`	`24`	`}`
`25`	`25`
	`26`	`+// CHECK: -package-name barpkg`
`26`	`27`	`// CHECK: public enum PubEnum {`
`27`	`28`	`// CHECK: case red, green`
`28`	`29`	`// CHECK: public static func == (a: Bar.PubEnum, b: Bar.PubEnum) -> Swift.Bool`