Merge pull request #71430 from apple/es-pkg-bypass

elsh · web-flow · commit 633051e31971 · 2024-02-14T14:25:11.000-08:00
Support bypassing resilience checks for package decls at use site in a package
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -2920,10 +2920,6 @@ class ValueDecl : public Decl {
   /// if the base declaration is \c open, the override might have to be too.
   bool hasOpenAccess(const DeclContext *useDC) const;
 
-  /// Returns whether this declaration should be treated as having the \c
-  /// package access level.
-  bool hasPackageAccess() const;
-
   /// FIXME: This is deprecated.
   bool isRecursiveValidation() const;
 
@@ -5846,13 +5842,6 @@ class AbstractStorageDecl : public ValueDecl {
                                    ModuleDecl *module,
                                    ResilienceExpansion expansion) const;
 
-  /// Should this declaration behave as if it must be accessed
-  /// resiliently, even when we're building a non-resilient module?
-  ///
-  /// This is used for diagnostics, because we do not want a behavior
-  /// change between builds with resilience enabled and disabled.
-  bool isFormallyResilient() const;
-
   /// Do we need to use resilient access patterns outside of this
   /// property's resilience domain?
   bool isResilient() const;
diff --git a/include/swift/AST/DeclContext.h b/include/swift/AST/DeclContext.h
@@ -530,6 +530,13 @@ class alignas(1 << DeclContextAlignInBits) DeclContext
   LLVM_READONLY
   PackageUnit *getPackageContext(bool lookupIfNotCurrent = false) const;
 
+  /// True if resilience checks can be bypassed within a package.
+  /// \p isForPackageDecl Bypassing only applies to package types
+  /// (possibly also public types later) if opted-in, client and defining module
+  /// are in the same package, and the defining module is a binary module.
+  LLVM_READONLY
+  bool bypassResilienceInPackage(bool isForPackageDecl) const;
+
   /// Returns the module context that contains this context.
   LLVM_READONLY
   ModuleDecl *getParentModule() const;
diff --git a/include/swift/AST/Module.h b/include/swift/AST/Module.h
@@ -480,6 +480,10 @@ class ModuleDecl
     return Identifier();
   }
 
+  bool inSamePackage(ModuleDecl *other) {
+    return !getPackageName().empty() && getPackageName() == other->getPackageName();
+  }
+
   /// Get the package associated with this module
   PackageUnit *getPackage() const { return Package; }
 
diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h
@@ -186,6 +186,9 @@ namespace swift {
     /// Disable API availability checking.
     bool DisableAvailabilityChecking = false;
 
+    /// Enable optimization to bypass resilience checks in a package
+    bool EnableBypassResilienceInPackage = false;
+
     /// Optimization mode for unavailable declarations.
     llvm::Optional<UnavailableDeclOptimization> UnavailableDeclOptimizationMode;
 
diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td
@@ -516,6 +516,10 @@ def unavailable_decl_optimization_EQ : Joined<["-"], "unavailable-decl-optimizat
            "value may be 'none' (no optimization) or 'complete' (code is not "
            "generated at all unavailable declarations)">;
 
+def package_bypass_resilience_optimization : Flag<["-"], "package-bypass-resilience-optimization">,
+  Flags<[FrontendOption]>,
+  HelpText<"Enable optimization to bypass resilience within a package">;
+
 def library_level : Separate<["-"], "library-level">,
   MetaVarName<"<level>">,
   Flags<[HelpHidden, FrontendOption, ModuleInterfaceOption]>,
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -2952,31 +2952,30 @@ bool Decl::isOutermostPrivateOrFilePrivateScope() const {
          !isInPrivateOrLocalContext(this);
 }
 
-bool AbstractStorageDecl::isFormallyResilient() const {
+bool AbstractStorageDecl::isResilient() const {
   // Check for an explicit @_fixed_layout attribute.
   if (getAttrs().hasAttribute<FixedLayoutAttr>())
     return false;
 
   // If we're an instance property of a nominal type, query the type.
-  auto *dc = getDeclContext();
   if (!isStatic())
-    if (auto *nominalDecl = dc->getSelfNominalTypeDecl())
+    if (auto *nominalDecl = getDeclContext()->getSelfNominalTypeDecl())
       return nominalDecl->isResilient();
 
   // Non-public global and static variables always have a
   // fixed layout.
-  if (!getFormalAccessScope(/*useDC=*/nullptr,
-                            /*treatUsableFromInlineAsPublic=*/true).isPublicOrPackage())
+  auto accessScope = getFormalAccessScope(/*useDC=*/nullptr,
+                                          /*treatUsableFromInlineAsPublic=*/true);
+  if (!accessScope.isPublicOrPackage())
     return false;
 
-  return true;
-}
-
-bool AbstractStorageDecl::isResilient() const {
-  if (!isFormallyResilient())
+  if (!getModuleContext()->isResilient())
     return false;
 
-  return getModuleContext()->isResilient();
+  // Allows bypassing resilience checks for package decls
+  // at use site within a package if opted in, whether the
+  // loaded module was built resiliently or not.
+  return !getDeclContext()->bypassResilienceInPackage(accessScope.isPackage());
 }
 
 bool AbstractStorageDecl::isResilient(ModuleDecl *M,
@@ -4207,13 +4206,6 @@ bool ValueDecl::hasOpenAccess(const DeclContext *useDC) const {
   return access == AccessLevel::Open;
 }
 
-bool ValueDecl::hasPackageAccess() const {
-  AccessLevel access =
-      getAdjustedFormalAccess(this, /*useDC*/ nullptr,
-                              /*treatUsableFromInlineAsPublic*/ false);
-  return access == AccessLevel::Package;
-}
-
 /// Given the formal access level for using \p VD, compute the scope where
 /// \p VD may be accessed, taking \@usableFromInline, \@testable imports,
 /// \@_spi imports, and enclosing access levels into account.
@@ -5054,8 +5046,14 @@ bool NominalTypeDecl::isFormallyResilient() const {
 bool NominalTypeDecl::isResilient() const {
   if (!isFormallyResilient())
     return false;
-
-  return getModuleContext()->isResilient();
+  if (!getModuleContext()->isResilient())
+    return false;
+  // Allows bypassing resilience checks for package decls
+  // at use site within a package if opted in, whether the
+  // loaded module was built resiliently or not.
+  auto accessScope = getFormalAccessScope(/*useDC=*/nullptr,
+                                          /*treatUsableFromInlineAsPublic=*/true);
+  return !getDeclContext()->bypassResilienceInPackage(accessScope.isPackage());
 }
 
 DestructorDecl *NominalTypeDecl::getValueTypeDestructor() {
@@ -6375,7 +6373,11 @@ bool EnumDecl::isFormallyExhaustive(const DeclContext *useDC) const {
   // Non-public, non-versioned enums are always exhaustive.
   AccessScope accessScope = getFormalAccessScope(/*useDC*/nullptr,
                                                  /*respectVersioned*/true);
-  if (!accessScope.isPublic())
+  // Both public and package enums should behave the same unless
+  // package enum is optimized with bypassing resilience checks.
+  if (!accessScope.isPublicOrPackage())
+    return true;
+  if (useDC && useDC->bypassResilienceInPackage(accessScope.isPackage()))
     return true;
 
   // All other checks are use-site specific; with no further information, the
diff --git a/lib/AST/DeclContext.cpp b/lib/AST/DeclContext.cpp
@@ -300,6 +300,17 @@ PackageUnit *DeclContext::getPackageContext(bool lookupIfNotCurrent) const {
   return nullptr;
 }
 
+bool DeclContext::bypassResilienceInPackage(bool isForPackageDecl) const {
+  // Bypassing resilience checks only applies to package types (and possibly
+  // public types in the same package in the future). Allowed only if opted-in
+  // for bypassing optimization, client and defining module are in the same
+  // package, and defining module is a binary module.
+  return isForPackageDecl &&
+         getASTContext().LangOpts.EnableBypassResilienceInPackage &&
+         getParentModule()->inSamePackage(getASTContext().MainModule) &&
+         !getParentModule()->isBuiltFromInterface();
+}
+
 ModuleDecl *DeclContext::getParentModule() const {
   // If the current context is PackageUnit, return the module
   // decl context pointing to the current context. This check
diff --git a/lib/Driver/ToolChains.cpp b/lib/Driver/ToolChains.cpp
@@ -299,6 +299,7 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
   inputArgs.AddLastArg(arguments, options::OPT_Rpass_missed_EQ);
   inputArgs.AddLastArg(arguments, options::OPT_suppress_warnings);
   inputArgs.AddLastArg(arguments, options::OPT_suppress_remarks);
+  inputArgs.AddLastArg(arguments, options::OPT_package_bypass_resilience_optimization);
   inputArgs.AddLastArg(arguments, options::OPT_profile_generate);
   inputArgs.AddLastArg(arguments, options::OPT_profile_use);
   inputArgs.AddLastArg(arguments, options::OPT_profile_coverage_mapping);
@@ -551,7 +552,7 @@ ToolChain::constructInvocation(const CompileJobAction &job,
   if (context.Args.hasArg(options::OPT_CrossModuleOptimization)) {
     Arguments.push_back("-cross-module-optimization");
   }
-                                 
+
   if (context.Args.hasArg(options::OPT_ExperimentalPerformanceAnnotations)) {
     Arguments.push_back("-experimental-performance-annotations");
   }
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
@@ -671,6 +671,8 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
   Opts.EnablePackageInterfaceLoad = Args.hasArg(OPT_experimental_package_interface_load) ||
                                     ::getenv("SWIFT_ENABLE_PACKAGE_INTERFACE_LOAD");
 
+  Opts.EnableBypassResilienceInPackage = Args.hasArg(OPT_package_bypass_resilience_optimization);
+
   Opts.DisableAvailabilityChecking |=
       Args.hasArg(OPT_disable_availability_checking);
   if (Args.hasArg(OPT_check_api_availability_only))
diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp
@@ -1889,7 +1889,10 @@ static bool canStorageUseTrivialDescriptor(SILGenModule &SGM,
     if (SGM.canStorageUseStoredKeyPathComponent(decl, expansion)) {
       // External modules can't directly access storage, unless this is a
       // property in a fixed-layout type.
-      return !decl->isFormallyResilient();
+      // Assert here as key path component cannot refer to a static var.
+      assert(!decl->isStatic());
+      // By this point, decl is a fixed layout or its enclosing type is non-resilient.
+      return true;
     }
 
     // If the type is computed and doesn't have a setter that's hidden from
diff --git a/lib/Sema/ImportResolution.cpp b/lib/Sema/ImportResolution.cpp
@@ -762,8 +762,7 @@ void UnboundImport::validateInterfaceWithPackageName(ModuleDecl *topLevelModule,
 
   // If source file is .swift or non-interface, show diags when importing an interface file
   ASTContext &ctx = topLevelModule->getASTContext();
-  if (!topLevelModule->getPackageName().empty() &&
-      topLevelModule->getPackageName().str() == ctx.LangOpts.PackageName &&
+  if (topLevelModule->inSamePackage(ctx.MainModule) &&
       topLevelModule->isBuiltFromInterface() &&
       !topLevelModule->getModuleSourceFilename().endswith(".package.swiftinterface")) {
       ctx.Diags.diagnose(import.module.getModulePath().front().Loc,
diff --git a/test/IRGen/package_resilience.swift b/test/IRGen/package_resilience.swift
@@ -1,20 +1,18 @@
 //
 // Unlike its counterparts in the other *_resilience.swift files, the goal is
 // for the package's component modules to all be considered within the same
-// resilience domain. This file ensures that we use direct access as much as
-// possible.
+// resilience domain. This file ensures that we use direct access to package
+// decls at use site as much as possible with -package-bypass-resilience-optimization.
 //
 
-// REQUIRES: rdar118947451
-
 // RUN: %empty-directory(%t)
 // RUN: %{python} %utils/chex.py < %s > %t/package_resilience.swift
 // RUN: %target-swift-frontend -package-name MyPkg -emit-module -enable-library-evolution -emit-module-path=%t/resilient_struct.swiftmodule -module-name=resilient_struct %S/Inputs/package_types/resilient_struct.swift
 // RUN: %target-swift-frontend -package-name MyPkg -emit-module -enable-library-evolution -emit-module-path=%t/resilient_enum.swiftmodule -module-name=resilient_enum -I %t %S/Inputs/package_types/resilient_enum.swift
 // RUN: %target-swift-frontend -package-name MyPkg -emit-module -enable-library-evolution -emit-module-path=%t/resilient_class.swiftmodule -module-name=resilient_class -I %t %S/Inputs/package_types/resilient_class.swift
-// RUN: %target-swift-frontend -package-name MyPkg -enable-objc-interop -I %t -emit-ir -enable-library-evolution %t/package_resilience.swift | %FileCheck %t/package_resilience.swift --check-prefixes=CHECK,CHECK-objc,CHECK-objc%target-ptrsize,CHECK-%target-ptrsize,CHECK-%target-cpu,CHECK-%target-import-type-objc-STABLE-ABI-%target-mandates-stable-abi,CHECK-%target-sdk-name -DINT=i%target-ptrsize -D#MDWORDS=7 -D#MDSIZE32=52 -D#MDSIZE64=80 -D#WORDSIZE=%target-alignment
-// RUN: %target-swift-frontend -package-name MyPkg -disable-objc-interop -I %t -emit-ir -enable-library-evolution %t/package_resilience.swift | %FileCheck %t/package_resilience.swift --check-prefixes=CHECK,CHECK-native,CHECK-native%target-ptrsize,CHECK-%target-ptrsize,CHECK-%target-cpu,CHECK-native-STABLE-ABI-%target-mandates-stable-abi,CHECK-%target-sdk-name -DINT=i%target-ptrsize -D#MDWORDS=4 -D#MDSIZE32=40 -D#MDSIZE64=56 -D#WORDSIZE=%target-alignment
-// RUN: %target-swift-frontend -package-name MyPkg -I %t -emit-ir -enable-library-evolution -O %t/package_resilience.swift -package-name MyPkg
+// RUN: %target-swift-frontend -package-name MyPkg -package-bypass-resilience-optimization -enable-objc-interop -I %t -emit-ir -enable-library-evolution %t/package_resilience.swift | %FileCheck %t/package_resilience.swift --check-prefixes=CHECK,CHECK-objc,CHECK-objc%target-ptrsize,CHECK-%target-ptrsize,CHECK-%target-cpu,CHECK-%target-import-type-objc-STABLE-ABI-%target-mandates-stable-abi,CHECK-%target-sdk-name -DINT=i%target-ptrsize -D#MDWORDS=7 -D#MDSIZE32=52 -D#MDSIZE64=80 -D#WORDSIZE=%target-alignment
+// RUN: %target-swift-frontend -package-name MyPkg -package-bypass-resilience-optimization -disable-objc-interop -I %t -emit-ir -enable-library-evolution %t/package_resilience.swift | %FileCheck %t/package_resilience.swift --check-prefixes=CHECK,CHECK-native,CHECK-native%target-ptrsize,CHECK-%target-ptrsize,CHECK-%target-cpu,CHECK-native-STABLE-ABI-%target-mandates-stable-abi,CHECK-%target-sdk-name -DINT=i%target-ptrsize -D#MDWORDS=4 -D#MDSIZE32=40 -D#MDSIZE64=56 -D#WORDSIZE=%target-alignment
+// RUN: %target-swift-frontend -package-name MyPkg -package-bypass-resilience-optimization -I %t -emit-ir -enable-library-evolution -O %t/package_resilience.swift -package-name MyPkg
 // REQUIRES: objc_codegen
 // REQUIRES: OS=macosx || OS=ios || OS=tvos || OS=watchos
 // REQUIRES: CPU=x86_64 || CPU=arm64
@@ -223,7 +221,7 @@ public func memoryLayoutDotSizeWithResilientStruct() -> Int {
 // CHECK-LABEL: define{{.*}} swiftcc {{i32|i64}} @"$s18package_resilience40memoryLayoutDotStrideWithResilientStructSiyF"()
 public func memoryLayoutDotStrideWithResilientStruct() -> Int {
   // CHECK: ret [[INT]] [[#WORDSIZE + WORDSIZE]]
-  return MemoryLayout<Size>.size
+  return MemoryLayout<Size>.stride
 }
 
 // CHECK-LABEL: define{{.*}} swiftcc {{i32|i64}} @"$s18package_resilience43memoryLayoutDotAlignmentWithResilientStructSiyF"()
diff --git a/test/Sema/package_resilience_bypass_exhaustive_switch.swift b/test/Sema/package_resilience_bypass_exhaustive_switch.swift

Original file line number	Diff line number	Diff line change
`@@ -480,6 +480,10 @@ class ModuleDecl`
`480`	`480`	`return Identifier();`
`481`	`481`	`}`
`482`	`482`
	`483`	`+ bool inSamePackage(ModuleDecl *other) {`
	`484`	`+ return !getPackageName().empty() && getPackageName() == other->getPackageName();`
	`485`	`+ }`
	`486`	`+`
`483`	`487`	`/// Get the package associated with this module`
`484`	`488`	`PackageUnit *getPackage() const { return Package; }`
`485`	`489`