From 2c50f4649836dd470004a5f640160fbb114063b2 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 9 Sep 2025 09:45:32 -0400 Subject: [PATCH 1/2] Don't crash if there's a request cycle with DefaultArgumentExprRequest. --- lib/AST/Decl.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) 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(); From 2fb9fc5ab23d382710c213e5ed450ac113a70918 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 9 Sep 2025 09:56:06 -0400 Subject: [PATCH 2/2] 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. --- include/swift/AST/Decl.h | 3 ++ lib/AST/Decl.cpp | 20 +++++-- lib/SIL/IR/SILFunctionType.cpp | 15 +++--- lib/SILGen/SILGenConcurrency.cpp | 4 +- lib/Sema/TypeCheckConcurrency.cpp | 8 +++ test/Concurrency/actor_defer.swift | 10 ++++ ...lated_nonsending_isolation_macro_sil.swift | 53 +++++++++++++++++++ 7 files changed, 99 insertions(+), 14 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/SIL/IR/SILFunctionType.cpp b/lib/SIL/IR/SILFunctionType.cpp index cae226d28d52a..1c38f32db8d31 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/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() { diff --git a/test/Concurrency/isolated_nonsending_isolation_macro_sil.swift b/test/Concurrency/isolated_nonsending_isolation_macro_sil.swift index eea5b80be8ad1..22eee57a346a7 100644 --- a/test/Concurrency/isolated_nonsending_isolation_macro_sil.swift +++ b/test/Concurrency/isolated_nonsending_isolation_macro_sil.swift @@ -19,3 +19,56 @@ 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 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)