From 3088b85c2c83020c7b311bd1ee006e13670e5c5a Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 9 Sep 2025 09:45:32 -0400 Subject: [PATCH 1/6] Don't crash if there's a request cycle with DefaultArgumentExprRequest. --- lib/AST/Decl.cpp | 13 ++++++++++++- .../8d73bb4170a0c447.swift | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/8d73bb4170a0c447.swift (81%) diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 0b3f42a541144..e8738d959f04f 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -9443,7 +9443,18 @@ void ParamDecl::setDefaultExpr(Expr *E) { } void ParamDecl::setTypeCheckedDefaultExpr(Expr *E) { - assert(E || getDefaultArgumentKind() == DefaultArgumentKind::Inherited); + // The type-checker will only produce a null expression here if the + // default argument is inherited, so if we're called with a null pointer + // in any other case, it must be from a request cycle. Don't crash; + // just wrap the original expression with an ErrorExpr and proceed. + if (!E && getDefaultArgumentKind() != DefaultArgumentKind::Inherited) { + auto *initExpr = getStructuralDefaultExpr(); + assert(initExpr); + auto &ctx = getASTContext(); + E = new (ctx) ErrorExpr(initExpr->getSourceRange(), ErrorType::get(ctx), + initExpr); + } + setDefaultExpr(E); auto *defaultInfo = DefaultValueAndFlags.getPointer(); diff --git a/validation-test/compiler_crashers_2/8d73bb4170a0c447.swift b/validation-test/compiler_crashers_2_fixed/8d73bb4170a0c447.swift similarity index 81% rename from validation-test/compiler_crashers_2/8d73bb4170a0c447.swift rename to validation-test/compiler_crashers_2_fixed/8d73bb4170a0c447.swift index d524ddf4279d5..7a79523337172 100644 --- a/validation-test/compiler_crashers_2/8d73bb4170a0c447.swift +++ b/validation-test/compiler_crashers_2_fixed/8d73bb4170a0c447.swift @@ -1,3 +1,3 @@ // {"kind":"typecheck","signature":"swift::ParamDecl::setTypeCheckedDefaultExpr(swift::Expr*)","signatureAssert":"Assertion failed: (E || getDefaultArgumentKind() == DefaultArgumentKind::Inherited), function setTypeCheckedDefaultExpr"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s func a(b= a( From 74496ea86dea0b23c4b28827980312b023939e2b Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 9 Sep 2025 14:08:49 -0400 Subject: [PATCH 2/6] Fix SILGen's computation of default argument generator isolation --- include/swift/SIL/SILDeclRef.h | 3 +++ lib/SIL/IR/SILDeclRef.cpp | 18 ++++++++++++++++++ lib/SIL/IR/SILFunctionType.cpp | 27 +-------------------------- lib/SILGen/SILGen.cpp | 30 ++---------------------------- 4 files changed, 24 insertions(+), 54 deletions(-) diff --git a/include/swift/SIL/SILDeclRef.h b/include/swift/SIL/SILDeclRef.h index 6a5a0377a159b..ce1f52a1c2f0c 100644 --- a/include/swift/SIL/SILDeclRef.h +++ b/include/swift/SIL/SILDeclRef.h @@ -38,6 +38,7 @@ namespace swift { enum class EffectsKind : uint8_t; class AbstractFunctionDecl; class AbstractClosureExpr; + class ActorIsolation; class ValueDecl; class FuncDecl; class ClosureExpr; @@ -508,6 +509,8 @@ struct SILDeclRef { return result; } + ActorIsolation getActorIsolation() const; + /// True if the decl ref references a thunk from a natively foreign /// declaration to Swift calling convention. bool isForeignToNativeThunk() const; diff --git a/lib/SIL/IR/SILDeclRef.cpp b/lib/SIL/IR/SILDeclRef.cpp index d3ab0b8b63d88..fcbbc694e9ef4 100644 --- a/lib/SIL/IR/SILDeclRef.cpp +++ b/lib/SIL/IR/SILDeclRef.cpp @@ -1921,3 +1921,21 @@ bool SILDeclRef::isCalleeAllocatedCoroutine() const { return getASTContext().SILOpts.CoroutineAccessorsUseYieldOnce2; } + +ActorIsolation SILDeclRef::getActorIsolation() const { + // Deallocating destructor is always nonisolated. Isolation of the deinit + // applies only to isolated deallocator and destroyer. + if (kind == SILDeclRef::Kind::Deallocator) { + return ActorIsolation::forNonisolated(false); + } + + // Default argument generators use the isolation of the initializer, + // not the general isolation of the function. + if (isDefaultArgGenerator()) { + auto param = getParameterAt(getDecl(), defaultArgIndex); + assert(param); + return param->getInitializerIsolation(); + } + + return getActorIsolationOfContext(getInnermostDeclContext()); +} diff --git a/lib/SIL/IR/SILFunctionType.cpp b/lib/SIL/IR/SILFunctionType.cpp index cae226d28d52a..0f5814ed9d04a 100644 --- a/lib/SIL/IR/SILFunctionType.cpp +++ b/lib/SIL/IR/SILFunctionType.cpp @@ -2412,32 +2412,7 @@ swift::getSILFunctionTypeActorIsolation(CanAnyFunctionType substFnInterfaceType, } if (constant) { - // TODO: It should to be possible to `getActorIsolation` if - // reference is to a decl instead of trying to get isolation - // from the reference kind, the attributes, or the context. - - if (constant->kind == SILDeclRef::Kind::Deallocator) { - return ActorIsolation::forNonisolated(false); - } - - if (auto *decl = constant->getAbstractFunctionDecl()) { - if (auto *nonisolatedAttr = - decl->getAttrs().getAttribute()) { - if (nonisolatedAttr->isNonSending()) - return ActorIsolation::forCallerIsolationInheriting(); - } - - if (decl->getAttrs().hasAttribute()) { - return ActorIsolation::forNonisolated(false /*unsafe*/); - } - } - - if (auto *closure = constant->getAbstractClosureExpr()) { - if (auto isolation = closure->getActorIsolation()) - return isolation; - } - - return getActorIsolationOfContext(constant->getInnermostDeclContext()); + return constant->getActorIsolation(); } if (substFnInterfaceType->hasExtInfo() && diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index 9dac2728954c4..60bbf50df7ba4 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -801,32 +801,6 @@ static bool isEmittedOnDemand(SILModule &M, SILDeclRef constant) { return false; } -static ActorIsolation getActorIsolationForFunction(SILFunction &fn) { - if (auto constant = fn.getDeclRef()) { - if (constant.kind == SILDeclRef::Kind::Deallocator) { - // Deallocating destructor is always nonisolated. Isolation of the deinit - // applies only to isolated deallocator and destroyer. - return ActorIsolation::forNonisolated(false); - } - - // If we have a closure expr, check if our type is - // nonisolated(nonsending). In that case, we use that instead. - if (auto *closureExpr = constant.getAbstractClosureExpr()) { - if (auto actorIsolation = closureExpr->getActorIsolation()) - return actorIsolation; - } - - // If we have actor isolation for our constant, put the isolation onto the - // function. If the isolation is unspecified, we do not return it. - if (auto isolation = - getActorIsolationOfContext(constant.getInnermostDeclContext())) - return isolation; - } - - // Otherwise, return for unspecified. - return ActorIsolation::forUnspecified(); -} - SILFunction *SILGenModule::getFunction(SILDeclRef constant, ForDefinition_t forDefinition) { // If we already emitted the function, return it. @@ -853,7 +827,7 @@ SILFunction *SILGenModule::getFunction(SILDeclRef constant, }); F->setDeclRef(constant); - F->setActorIsolation(getActorIsolationForFunction(*F)); + F->setActorIsolation(constant.getActorIsolation()); assert(F && "SILFunction should have been defined"); @@ -1384,7 +1358,7 @@ void SILGenModule::preEmitFunction(SILDeclRef constant, SILFunction *F, F->setDeclRef(constant); // Set our actor isolation. - F->setActorIsolation(getActorIsolationForFunction(*F)); + F->setActorIsolation(constant.getActorIsolation()); LLVM_DEBUG(llvm::dbgs() << "lowering "; F->printName(llvm::dbgs()); From 5fb314b475215eb3fdc79f17769cc9180d85ebbc Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 9 Sep 2025 11:38:18 -0400 Subject: [PATCH 3/6] At the SIL level, allow synchronous functions to be nonisolated(nonsending). --- lib/SIL/IR/SILFunctionType.cpp | 15 ++++---- lib/SILGen/SILGenConcurrency.cpp | 4 +- lib/SILGen/SILGenExpr.cpp | 16 ++++++-- test/SILGen/isolated_default_arguments.swift | 40 ++++++++++++++++++++ test/SILGen/objc_async_from_swift.swift | 4 +- 5 files changed, 65 insertions(+), 14 deletions(-) diff --git a/lib/SIL/IR/SILFunctionType.cpp b/lib/SIL/IR/SILFunctionType.cpp index 0f5814ed9d04a..8ba7870dec67e 100644 --- a/lib/SIL/IR/SILFunctionType.cpp +++ b/lib/SIL/IR/SILFunctionType.cpp @@ -1261,6 +1261,11 @@ class Conventions { ConventionsKind getKind() const { return kind; } + bool hasCallerIsolationParameter() const { + return kind == ConventionsKind::Default || + kind == ConventionsKind::Deallocator; + } + virtual ParameterConvention getIndirectParameter(unsigned index, const AbstractionPattern &type, @@ -1700,14 +1705,10 @@ class DestructureInputs { }; } - // If we are an async function that is unspecified or nonisolated, insert an - // isolated parameter if NonisolatedNonsendingByDefault is enabled. - // - // NOTE: The parameter is not inserted for async functions imported - // from ObjC because they are handled in a special way that doesn't - // require it. + // If the function has nonisolated(nonsending) isolation, insert the + // implicit isolation parameter. if (IsolationInfo && IsolationInfo->isCallerIsolationInheriting() && - extInfoBuilder.isAsync() && !Foreign.async) { + Convs.hasCallerIsolationParameter()) { auto actorProtocol = TC.Context.getProtocol(KnownProtocolKind::Actor); auto actorType = ExistentialType::get(actorProtocol->getDeclaredInterfaceType()); diff --git a/lib/SILGen/SILGenConcurrency.cpp b/lib/SILGen/SILGenConcurrency.cpp index b6500bd388a9f..64455af576313 100644 --- a/lib/SILGen/SILGenConcurrency.cpp +++ b/lib/SILGen/SILGenConcurrency.cpp @@ -190,7 +190,7 @@ void SILGenFunction::emitExpectedExecutorProlog() { } case ActorIsolation::CallerIsolationInheriting: - assert(F.isAsync()); + assert(F.isAsync() || F.isDefer()); setExpectedExecutorForParameterIsolation(*this, actorIsolation); break; @@ -255,7 +255,7 @@ void SILGenFunction::emitExpectedExecutorProlog() { RegularLocation::getDebugOnlyLocation(F.getLocation(), getModule()), executor, /*mandatory*/ false); - } else { + } else if (wantDataRaceChecks) { // For a synchronous function, check that we're on the same executor. // Note: if we "know" that the code is completely Sendable-safe, this // is unnecessary. The type checker will need to make this determination. diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index c6385b4cb2aa4..8f03e26c7d8f5 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -2950,12 +2950,22 @@ SILGenFunction::emitApplyOfDefaultArgGenerator(SILLocation loc, ResultPlanBuilder::computeResultPlan(*this, calleeTypeInfo, loc, C); ArgumentScope argScope(*this, loc); - SmallVector captures; + SmallVector args; + + // Add the implicit leading isolation argument for a + // nonisolated(nonsending) default argument generator. + if (fnType->maybeGetIsolatedParameter()) { + auto executor = emitExpectedExecutor(loc); + args.push_back(emitActorInstanceIsolation( + loc, executor, executor.getType().getASTType())); + } + + // If there are captures, those come at the end. emitCaptures(loc, generator, CaptureEmission::ImmediateApplication, - captures); + args); return emitApply(std::move(resultPtr), std::move(argScope), loc, fnRef, subs, - captures, calleeTypeInfo, ApplyOptions(), C, std::nullopt); + args, calleeTypeInfo, ApplyOptions(), C, std::nullopt); } RValue SILGenFunction::emitApplyOfStoredPropertyInitializer( diff --git a/test/SILGen/isolated_default_arguments.swift b/test/SILGen/isolated_default_arguments.swift index 47c8fd581daaf..627904e6330e7 100644 --- a/test/SILGen/isolated_default_arguments.swift +++ b/test/SILGen/isolated_default_arguments.swift @@ -28,6 +28,10 @@ func main_actor_int_pair() -> (Int, Int) { return (0,0) } +func make_int(isolated isolation: (any Actor)? = #isolation) -> Int { + return 0 +} + // This test breaks because the intermediate argument is `nil`, which // we treat as non-isolated. @MainActor @@ -121,3 +125,39 @@ func tupleIsolatedDefaultArg(x: (Int,Int) = main_actor_int_pair(), func testTupleIsolatedDefaultArg() async { await tupleIsolatedDefaultArg(y: 0) } + +// When a function is caller-isolated, its default argument generators +// should probably also be caller-isolated and forward their isolation +// properly when #isolation is used. Currently, however, that's not what +// we're doing, so test for the current behavior. + +nonisolated(nonsending) +func callerIsolatedDefaultArg(x: Int = make_int()) async {} + +@MainActor +func useCallerIsolatedDefaultArg() async { + await callerIsolatedDefaultArg() +} + +// Check that the default argument generator isn't caller-isolated. +// CHECK-LABEL: // default argument 0 of test.callerIsolatedDefaultArg +// CHECK-NEXT: // Isolation: unspecified +// CHECK-NEXT: sil hidden [ossa] @$s4test24callerIsolatedDefaultArg1xySi_tYaFfA_ : +// CHECK: bb0: +// Check that we provide a nil isolation for #isolation +// CHECK-NEXT: [[NIL_ISOLATION:%.*]] = enum $Optional, #Optional.none +// CHECK-NEXT: // function_ref test.make_int +// CHECK-NEXT: [[FN:%.*]] = function_ref @$s4test8make_int8isolatedSiScA_pSg_tF : +// CHECK-NEXT: [[RESULT:%.*]] = apply [[FN]]([[NIL_ISOLATION]]) +// CHECK-NEXT: return [[RESULT]] + +// Check that we pass the right isolation to the generator. +// CHECK-LABEL: sil hidden [ossa] @$s4test27useCallerIsolatedDefaultArgyyYaF : +// Get the main actor reference. +// CHECK: [[GET_MAIN_ACTOR:%.*]] = function_ref @$sScM6sharedScMvgZ : +// CHECK-NEXT: [[T0:%.*]] = apply [[GET_MAIN_ACTOR]]( +// CHECK-NEXT: [[MAIN_ACTOR:%.*]] = begin_borrow [[T0]] +// Call the accessor. +// CHECK: // function_ref default argument 0 of +// CHECK-NEXT: [[GEN:%.*]] = function_ref @$s4test24callerIsolatedDefaultArg1xySi_tYaFfA_ : +// CHECK-NEXT: [[ARG:%.*]] = apply [[GEN]]() diff --git a/test/SILGen/objc_async_from_swift.swift b/test/SILGen/objc_async_from_swift.swift index 49bd3b9705fbb..6e8b2fc9b781c 100644 --- a/test/SILGen/objc_async_from_swift.swift +++ b/test/SILGen/objc_async_from_swift.swift @@ -466,7 +466,7 @@ func testAutoclosureInStaticMethod() { // // Get standard. // CHECK: [[METATYPE:%.*]] = metatype $@objc_metatype SlowServer.Type - // CHECK: [[GET_STANDARD_FUNC:%.*]] = objc_method %1 : $@objc_metatype SlowServer.Type, #SlowServer.standard!getter.foreign : (SlowServer.Type) -> () -> SlowServer, $@convention(objc_method) (@objc_metatype SlowServer.Type) -> @autoreleased SlowServer + // CHECK: [[GET_STANDARD_FUNC:%.*]] = objc_method [[METATYPE]] : $@objc_metatype SlowServer.Type, #SlowServer.standard!getter.foreign : (SlowServer.Type) -> () -> SlowServer, $@convention(objc_method) (@objc_metatype SlowServer.Type) -> @autoreleased SlowServer // CHECK: [[STANDARD:%.*]] = apply [[GET_STANDARD_FUNC]]([[METATYPE]]) // // Then grab value. @@ -592,7 +592,7 @@ func testAutoclosureInStaticMethod() { // // Get standard. // CHECK: [[METATYPE:%.*]] = metatype $@objc_metatype SlowServer.Type - // CHECK: [[GET_STANDARD_FUNC:%.*]] = objc_method %1 : $@objc_metatype SlowServer.Type, #SlowServer.standard!getter.foreign : (SlowServer.Type) -> () -> SlowServer, $@convention(objc_method) (@objc_metatype SlowServer.Type) -> @autoreleased SlowServer + // CHECK: [[GET_STANDARD_FUNC:%.*]] = objc_method [[METATYPE]] : $@objc_metatype SlowServer.Type, #SlowServer.standard!getter.foreign : (SlowServer.Type) -> () -> SlowServer, $@convention(objc_method) (@objc_metatype SlowServer.Type) -> @autoreleased SlowServer // CHECK: [[STANDARD:%.*]] = apply [[GET_STANDARD_FUNC]]([[METATYPE]]) // // Then grab value. From 367520cd3f763ef0c28c981b5342d3cda31bd732 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 9 Sep 2025 11:53:46 -0400 Subject: [PATCH 4/6] Fix two bugs with the isolation of defer bodies. The first bug is that we weren't computing isolation correctly for nested defers. This is an unlikely pattern of code, but it's good to fix. The second bug is that getActorIsolationOfContext was looking through defers, but getActorIsolation itself was not. This was causing defer bodies to be emitted in SILGen without an isolation parameter, which meant that #isolation could not possibly provide the right value. Fixing this involves teaching SILGen that non-async functions can have nonisolated(nonsending) isolation, but that's relatively straightforward. This commit doesn't fix #isolation or adequately test SILGen, but that'll be handled in a follow-up. --- include/swift/AST/Decl.h | 3 +++ lib/AST/Decl.cpp | 20 +++++++++++++++----- lib/Sema/TypeCheckConcurrency.cpp | 8 ++++++++ test/Concurrency/actor_defer.swift | 10 ++++++++++ 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index bf3794041fd9a..aac18f6a304f5 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -3285,6 +3285,9 @@ class ValueDecl : public Decl { /// `AbstractStorageDecl`, returns `false`. bool isAsync() const; + /// Returns whether this function represents a defer body. + bool isDeferBody() const; + private: bool isObjCDynamic() const { return isObjC() && isDynamic(); diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index e8738d959f04f..1932e533ce04a 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -11879,6 +11879,12 @@ PrecedenceGroupDecl *InfixOperatorDecl::getPrecedenceGroup() const { nullptr); } +bool ValueDecl::isDeferBody() const { + if (auto fn = dyn_cast(this)) + return fn->isDeferBody(); + return false; +} + bool FuncDecl::isDeferBody() const { return getBaseIdentifier() == getASTContext().getIdentifier("$defer"); } @@ -12072,12 +12078,16 @@ ActorIsolation swift::getActorIsolationOfContext( getClosureActorIsolation) { auto &ctx = dc->getASTContext(); auto dcToUse = dc; - // Defer bodies share actor isolation of their enclosing context. - if (auto FD = dyn_cast(dcToUse)) { - if (FD->isDeferBody()) { - dcToUse = FD->getDeclContext(); - } + + // Defer bodies share the actor isolation of their enclosing context. + // We don't actually have to do this check here because + // getActorIsolation does consider it already, but it's nice to + // avoid some extra request evaluation in a trivial case. + while (auto FD = dyn_cast(dcToUse)) { + if (!FD->isDeferBody()) break; + dcToUse = FD->getDeclContext(); } + if (auto *vd = dyn_cast_or_null(dcToUse->getAsDecl())) return getActorIsolation(vd); diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index dd59321e7c7cc..6c8657b01700e 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -6353,6 +6353,14 @@ static bool shouldSelfIsolationOverrideDefault( static InferredActorIsolation computeActorIsolation(Evaluator &evaluator, ValueDecl *value) { + // Defer bodies share the actor isolation of their enclosing context. + if (value->isDeferBody()) { + return { + getActorIsolationOfContext(value->getDeclContext()), + IsolationSource() + }; + } + // If this declaration has actor-isolated "self", it's isolated to that // actor. if (evaluateOrDefault(evaluator, HasIsolatedSelfRequest{value}, false)) { diff --git a/test/Concurrency/actor_defer.swift b/test/Concurrency/actor_defer.swift index ceb92b4c1b72c..ab97f7aed86d5 100644 --- a/test/Concurrency/actor_defer.swift +++ b/test/Concurrency/actor_defer.swift @@ -36,6 +36,16 @@ func testNonDefer_negative() { // CHECK-NEXT: function_ref // CHECK-NEXT: apply +@MainActor func testGlobalActor_nested_positive() { + defer { + defer { + requiresMainActor() + } + doSomething() + } + doSomething() +} + #if NEGATIVES // expected-note @+1 {{add '@MainActor' to make global function 'testGlobalActor_negative()' part of global actor 'MainActor'}} func testGlobalActor_negative() { From c7cad4ba545d3a8bcc292baade2e853037224346 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 9 Sep 2025 14:15:05 -0400 Subject: [PATCH 5/6] Add a function for querying the isolation of an Initializer. --- include/swift/AST/ActorIsolation.h | 5 ++ lib/AST/Decl.cpp | 86 +++++++++++++++++++++--------- 2 files changed, 65 insertions(+), 26 deletions(-) diff --git a/include/swift/AST/ActorIsolation.h b/include/swift/AST/ActorIsolation.h index af036c944d50a..8bb4a06026259 100644 --- a/include/swift/AST/ActorIsolation.h +++ b/include/swift/AST/ActorIsolation.h @@ -28,6 +28,7 @@ class raw_ostream; namespace swift { class DeclContext; +class Initializer; class ModuleDecl; class VarDecl; class NominalTypeDecl; @@ -434,6 +435,10 @@ InferredActorIsolation getInferredActorIsolation(ValueDecl *value); ActorIsolation __AbstractClosureExpr_getActorIsolation(AbstractClosureExpr *CE); +/// Determine how the given initialization context is isolated. +ActorIsolation getActorIsolation(Initializer *init, + bool ignoreDefaultArguments = false); + /// Determine how the given declaration context is isolated. /// \p getClosureActorIsolation allows the specification of actor isolation for /// closures that haven't been saved been saved to the AST yet. This is useful diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 1932e533ce04a..ca6d171d0d8c4 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -12076,7 +12076,6 @@ ActorIsolation swift::getActorIsolationOfContext( DeclContext *dc, llvm::function_ref getClosureActorIsolation) { - auto &ctx = dc->getASTContext(); auto dcToUse = dc; // Defer bodies share the actor isolation of their enclosing context. @@ -12091,44 +12090,79 @@ ActorIsolation swift::getActorIsolationOfContext( if (auto *vd = dyn_cast_or_null(dcToUse->getAsDecl())) return getActorIsolation(vd); - // In the context of the initializing or default-value expression of a - // stored property: - // - For a static stored property, the isolation matches the VarDecl. - // Static properties are initialized upon first use, so the isolation - // of the initializer must match the isolation required to access the - // property. - // - For a field of a nominal type, the expression can require the same - // actor isolation as the field itself. That default expression may only - // be used from inits that meet the required isolation. - if (auto *var = dcToUse->getNonLocalVarDecl()) { - // If IsolatedDefaultValues are enabled, treat this context as having - // unspecified isolation. We'll compute the required isolation for - // the initializer and validate that it matches the isolation of the - // var itself in the DefaultInitializerIsolation request. - if (ctx.LangOpts.hasFeature(Feature::IsolatedDefaultValues)) - return ActorIsolation::forUnspecified(); - - return getActorIsolation(var); - } - if (auto *closure = dyn_cast(dcToUse)) { return getClosureActorIsolation(closure); } + if (auto *init = dyn_cast(dcToUse)) { + // FIXME: force default argument initializers to report a meaningless + // isolation in order to break a bunch of cycles with the way that + // isolation is computed for them. + return getActorIsolation(init, /*ignoreDefaultArguments*/ true); + } + if (isa(dcToUse)) { + auto &ctx = dc->getASTContext(); if (dcToUse->isAsyncContext() || - dcToUse->getASTContext().LangOpts.StrictConcurrencyLevel >= - StrictConcurrency::Complete) { - if (Type mainActor = dcToUse->getASTContext().getMainActorType()) + ctx.LangOpts.StrictConcurrencyLevel >= StrictConcurrency::Complete) { + if (Type mainActor = ctx.getMainActorType()) return ActorIsolation::forGlobalActor(mainActor) - .withPreconcurrency( - !dcToUse->getASTContext().isSwiftVersionAtLeast(6)); + .withPreconcurrency(!ctx.isSwiftVersionAtLeast(6)); } } return ActorIsolation::forUnspecified(); } +ActorIsolation swift::getActorIsolation(Initializer *init, + bool ignoreDefaultArguments) { + switch (init->getInitializerKind()) { + case InitializerKind::PatternBinding: + // In the context of the initializing or default-value expression of a + // stored property: + // - For a static stored property, the isolation matches the VarDecl. + // Static properties are initialized upon first use, so the isolation + // of the initializer must match the isolation required to access the + // property. + // - For a field of a nominal type, the expression can require the same + // actor isolation as the field itself. That default expression may only + // be used from inits that meet the required isolation. + if (auto *var = init->getNonLocalVarDecl()) { + auto &ctx = var->getASTContext(); + + // If IsolatedDefaultValues are enabled, treat this context as having + // unspecified isolation. We'll compute the required isolation for + // the initializer and validate that it matches the isolation of the + // var itself in the DefaultInitializerIsolation request. + if (ctx.LangOpts.hasFeature(Feature::IsolatedDefaultValues)) + return ActorIsolation::forUnspecified(); + + return getActorIsolation(var); + } + + return ActorIsolation::forUnspecified(); + + case InitializerKind::DefaultArgument: { + auto defArgInit = cast(init); + + // A hack when used from getActorIsolationOfContext to maintain the + // current behavior and avoid request cycles. + if (ignoreDefaultArguments) + return ActorIsolation::forUnspecified(); + + auto fn = cast(defArgInit->getParent()->getAsDecl()); + auto param = getParameterAt(fn, defArgInit->getIndex()); + assert(param); + return param->getInitializerIsolation(); + } + + case InitializerKind::PropertyWrapper: + case InitializerKind::CustomAttribute: + return ActorIsolation::forUnspecified(); + } + llvm_unreachable("bad initializer kind"); +} + bool swift::isSameActorIsolated(ValueDecl *value, DeclContext *dc) { auto valueIsolation = getActorIsolation(value); auto dcIsolation = getActorIsolationOfContext(dc); From 8301d5383b89147f7bd86ac1f98920aefefe35ff Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 9 Sep 2025 14:15:59 -0400 Subject: [PATCH 6/6] Fix the evaluation of #isolation in non-AFD contexts, most importantly closures. The fixes for initializers are just setting the stage for doing this properly: we should be able to just change the isolation computation in Sema and fix up the tests. --- lib/SILGen/SILGenExpr.cpp | 51 ++++++++------ ...lated_nonsending_isolation_macro_sil.swift | 66 +++++++++++++++++++ 2 files changed, 98 insertions(+), 19 deletions(-) diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 8f03e26c7d8f5..0dd1b60351946 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -7393,15 +7393,38 @@ RValue RValueEmitter::visitMacroExpansionExpr(MacroExpansionExpr *E, return RValue(); } +/// getActorIsolationOfContext doesn't return the right isolation for +/// initializer contexts. Fixing that is a lot of work, so just +/// hack around it for now here. +static ActorIsolation getRealActorIsolationOfContext(DeclContext *DC) { + if (auto init = dyn_cast(DC)) { + return getActorIsolation(init); + } else { + return getActorIsolationOfContext(DC); + } +} + RValue RValueEmitter::visitCurrentContextIsolationExpr( CurrentContextIsolationExpr *E, SGFContext C) { - auto afd = - dyn_cast_or_null(SGF.F.getDeclRef().getDecl()); - if (afd) { - auto isolation = getActorIsolation(afd); - auto ctor = dyn_cast_or_null(afd); + + auto isolation = getRealActorIsolationOfContext(SGF.FunctionDC); + + if (isolation == ActorIsolation::CallerIsolationInheriting) { + auto *isolatedArg = SGF.F.maybeGetIsolatedArgument(); + assert(isolatedArg && + "Caller Isolation Inheriting without isolated parameter"); + ManagedValue isolatedMV; + if (isolatedArg->getOwnershipKind() == OwnershipKind::Guaranteed) { + isolatedMV = ManagedValue::forBorrowedRValue(isolatedArg); + } else { + isolatedMV = ManagedValue::forUnmanagedOwnedValue(isolatedArg); + } + return RValue(SGF, E, isolatedMV); + } + + if (isolation == ActorIsolation::ActorInstance) { + auto ctor = dyn_cast(SGF.FunctionDC); if (ctor && ctor->isDesignatedInit() && - isolation == ActorIsolation::ActorInstance && isolation.getActorInstance() == ctor->getImplicitSelfDecl()) { // If we are in an actor initializer that is isolated to self, the // current isolation is flow-sensitive; use that instead of the @@ -7410,21 +7433,11 @@ RValue RValueEmitter::visitCurrentContextIsolationExpr( SGF.emitFlowSensitiveSelfIsolation(E, isolation); return RValue(SGF, E, isolationValue); } - - if (isolation == ActorIsolation::CallerIsolationInheriting) { - auto *isolatedArg = SGF.F.maybeGetIsolatedArgument(); - assert(isolatedArg && - "Caller Isolation Inheriting without isolated parameter"); - ManagedValue isolatedMV; - if (isolatedArg->getOwnershipKind() == OwnershipKind::Guaranteed) { - isolatedMV = ManagedValue::forBorrowedRValue(isolatedArg); - } else { - isolatedMV = ManagedValue::forUnmanagedOwnedValue(isolatedArg); - } - return RValue(SGF, E, isolatedMV); - } } + // Otherwise, just trust the underlying expression. We really don't + // need to do this at all --- we assume we can produce the isolation + // value from scratch in other situations --- but whatever. return visit(E->getActor(), C); } diff --git a/test/Concurrency/isolated_nonsending_isolation_macro_sil.swift b/test/Concurrency/isolated_nonsending_isolation_macro_sil.swift index eea5b80be8ad1..4144401e32dde 100644 --- a/test/Concurrency/isolated_nonsending_isolation_macro_sil.swift +++ b/test/Concurrency/isolated_nonsending_isolation_macro_sil.swift @@ -19,3 +19,69 @@ func take(iso: (any Actor)?) {} // CHECK: apply [[FUNC]]([[ACTOR]]) : $@convention(thin) (@guaranteed Optional) -> () // CHECK: release_value [[ACTOR]] // CHECK: } // end sil function '$s39isolated_nonsending_isolation_macro_sil21nonisolatedNonsendingyyYaF' + +// Check that we emit #isolation correctly in closures. +// CHECK-LABEL: // closure #1 in containsClosure() +// CHECK-NEXT: // Isolation: caller_isolation_inheriting +// CHECK: bb0(%0 : $Optional): +// CHECK-NEXT: // function_ref take(iso:) +// CHECK-NEXT: [[FN:%.*]] = function_ref @ +// CHECK-NEXT: apply [[FN]](%0) +func containsClosure() { + let closure: nonisolated(nonsending) () async -> Void = { + take(iso: #isolation) + } +} + +// Check that we emit #isolation correctly in defer bodies. +nonisolated(nonsending) +func hasDefer() async { + defer { + take(iso: #isolation) + } + do {} +} +// CHECK-LABEL: sil hidden @$s39isolated_nonsending_isolation_macro_sil8hasDeferyyYaF : +// CHECK: bb0(%0 : $Optional): +// CHECK: // function_ref $defer +// CHECK-NEXT: [[DEFER:%.*]] = function_ref +// CHECK-NEXT: apply [[DEFER]](%0) + +// CHECK-LABEL: // $defer #1 () in hasDefer() +// CHECK-NEXT: // Isolation: caller_isolation_inheriting +// CHECK: bb0(%0 : $Optional): +// CHECK-NEXT: // function_ref take(iso:) +// CHECK-NEXT: [[FN:%.*]] = function_ref @ +// CHECK-NEXT: apply [[FN]](%0) + +// Check that we emit #isolation correctly in nested defer bodies. +nonisolated(nonsending) +func hasNestedDefer() async { + defer { + defer { + take(iso: #isolation) + } + do {} + } + do {} +} + +// CHECK-LABEL: sil hidden @$s39isolated_nonsending_isolation_macro_sil14hasNestedDeferyyYaF : +// CHECK: bb0(%0 : $Optional): +// CHECK: // function_ref $defer #1 () in hasNestedDefer() +// CHECK-NEXT: [[DEFER:%.*]] = function_ref +// CHECK-NEXT: apply [[DEFER]](%0) + +// CHECK-LABEL: // $defer #1 () in hasNestedDefer() +// CHECK-NEXT: // Isolation: caller_isolation_inheriting +// CHECK: bb0(%0 : $Optional): +// CHECK: // function_ref $defer #1 () in $defer #1 () in hasNestedDefer() +// CHECK-NEXT: [[DEFER:%.*]] = function_ref +// CHECK-NEXT: apply [[DEFER]](%0) + +// CHECK-LABEL: // $defer #1 () in $defer #1 () in hasNestedDefer() +// CHECK-NEXT: // Isolation: caller_isolation_inheriting +// CHECK: bb0(%0 : $Optional): +// CHECK-NEXT: // function_ref take(iso:) +// CHECK-NEXT: [[FN:%.*]] = function_ref @ +// CHECK-NEXT: apply [[FN]](%0)