Skip to content

Support bypassing resilience checks for package decls at use site in a package #71430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions include/swift/AST/DeclContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions include/swift/AST/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

Expand Down
3 changes: 3 additions & 0 deletions include/swift/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 4 additions & 0 deletions include/swift/Option/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -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]>,
Expand Down
44 changes: 23 additions & 21 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions lib/AST/DeclContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/Driver/ToolChains.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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");
}
Expand Down
2 changes: 2 additions & 0 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
5 changes: 4 additions & 1 deletion lib/SILGen/SILGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions lib/Sema/ImportResolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 6 additions & 8 deletions test/IRGen/package_resilience.swift
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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"()
Expand Down
Loading