From 5b5ac983544e4936c4adeaa8b8aa10743e3112cc Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 12 Apr 2024 13:16:28 -0400 Subject: [PATCH 1/5] Concurrency: Don't walk into local types when checking isolation --- lib/Sema/TypeCheckConcurrency.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 52a8ee024f86e..fb9ae7ae46c1d 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2611,6 +2611,13 @@ namespace { } PreWalkAction walkToDeclPre(Decl *decl) override { + // Don't walk into local types because nothing in them can + // change the outcome of our analysis, and we don't want to + // assume things there have been type checked yet. + if (isa(decl)) { + return Action::SkipChildren(); + } + if (auto func = dyn_cast(decl)) { if (func->isLocalContext()) { checkLocalCaptures(func); From f3e1d8766ea7a2e8db0bfa606a283a8a2041ead9 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 12 Apr 2024 14:46:23 -0400 Subject: [PATCH 2/5] Concurrency: Fix incorrect isLocalContext() check --- lib/Sema/TypeCheckConcurrency.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index fb9ae7ae46c1d..133be38fa86e3 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2619,7 +2619,7 @@ namespace { } if (auto func = dyn_cast(decl)) { - if (func->isLocalContext()) { + if (func->getDeclContext()->isLocalContext()) { checkLocalCaptures(func); } From 4f15b471d550f31c72eca2628a0093fa7f6ec00b Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 12 Apr 2024 13:40:32 -0400 Subject: [PATCH 3/5] Concurrency: Contextualize top-level code before checking actor isolation --- lib/Sema/TypeCheckStmt.cpp | 6 ++---- lib/Sema/TypeChecker.cpp | 1 - 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp index 2f7ed04579e6d..01d0370e4415d 100644 --- a/lib/Sema/TypeCheckStmt.cpp +++ b/lib/Sema/TypeCheckStmt.cpp @@ -2994,12 +2994,10 @@ bool TypeChecker::typeCheckTapBody(TapExpr *expr, DeclContext *DC) { } void TypeChecker::typeCheckTopLevelCodeDecl(TopLevelCodeDecl *TLCD) { - // We intentionally use typeCheckStmt instead of typeCheckBody here - // because we want to contextualize all the TopLevelCode - // declarations simultaneously. BraceStmt *Body = TLCD->getBody(); - StmtChecker(TLCD).typeCheckStmt(Body); + StmtChecker(TLCD).typeCheckBody(Body); TLCD->setBody(Body); + checkTopLevelActorIsolation(TLCD); checkTopLevelEffects(TLCD); performTopLevelDeclDiagnostics(TLCD); diff --git a/lib/Sema/TypeChecker.cpp b/lib/Sema/TypeChecker.cpp index 3609bd5449fba..d0550ae9f82e8 100644 --- a/lib/Sema/TypeChecker.cpp +++ b/lib/Sema/TypeChecker.cpp @@ -284,7 +284,6 @@ TypeCheckSourceFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const { for (auto D : SF->getTopLevelDecls()) { if (auto *TLCD = dyn_cast(D)) { TypeChecker::typeCheckTopLevelCodeDecl(TLCD); - TypeChecker::contextualizeTopLevelCode(TLCD); } else { TypeChecker::typeCheckDecl(D); } From ff89c716ccc84db8a389346c68a67c92042344db Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 12 Apr 2024 15:04:11 -0400 Subject: [PATCH 4/5] Concurrency: Skip Patterns when checking isolation Due to a quirk of the ASTVisitor, we would then visit accessors twice, the first time as part of the PatternBindingDecl inside the TopLevelCodeDecl, and at that point the accessor body has not yet been type checked. --- lib/Sema/TypeCheckConcurrency.cpp | 7 +++++++ .../sendable_checking_script_mode.swift | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 test/Concurrency/sendable_checking_script_mode.swift diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 133be38fa86e3..a57e8930966e2 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2610,6 +2610,13 @@ namespace { return MacroWalking::Expansion; } + PreWalkResult walkToPatternPre(Pattern *pattern) override { + // Walking into patterns leads to nothing good because then we + // end up visiting the AccessorDecls of a top-level + // PatternBindingDecl twice. + return Action::SkipNode(pattern); + } + PreWalkAction walkToDeclPre(Decl *decl) override { // Don't walk into local types because nothing in them can // change the outcome of our analysis, and we don't want to diff --git a/test/Concurrency/sendable_checking_script_mode.swift b/test/Concurrency/sendable_checking_script_mode.swift new file mode 100644 index 0000000000000..1a6ae26825ee1 --- /dev/null +++ b/test/Concurrency/sendable_checking_script_mode.swift @@ -0,0 +1,17 @@ +// RUN: %target-typecheck-verify-swift -swift-version 6 + +class NonSendable {} // expected-note 2{{class 'NonSendable' does not conform to the 'Sendable' protocol}} + +var testLocalCaptures: Int { + let ns = NonSendable() + + @Sendable func localFunc() -> NonSendable { + return ns // expected-error {{capture of 'ns' with non-sendable type 'NonSendable' in a `@Sendable` local function}} + } + + let _: @Sendable () -> NonSendable = { + return ns // expected-error {{capture of 'ns' with non-sendable type 'NonSendable' in a `@Sendable` closure}} + } + + return 3 +} \ No newline at end of file From 57121755069f4aa3a7b5a628308210c66ddb2180 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 12 Apr 2024 16:39:28 -0400 Subject: [PATCH 5/5] Concurrency: Contextualize initializers before checking actor isolation Otherwise, we'll miss captures inside closures inside stored property initializer expressions. --- lib/Sema/TypeCheckConcurrency.cpp | 4 +-- lib/Sema/TypeCheckDeclPrimary.cpp | 11 +++++++- lib/Sema/TypeCheckStorage.cpp | 1 - .../sendable_checking_captures.swift | 25 +++++++++++++++++++ .../sendable_checking_script_mode.swift | 17 ------------- 5 files changed, 36 insertions(+), 22 deletions(-) create mode 100644 test/Concurrency/sendable_checking_captures.swift delete mode 100644 test/Concurrency/sendable_checking_script_mode.swift diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index a57e8930966e2..3894ada9b60dc 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -5369,11 +5369,9 @@ DefaultInitializerIsolation::evaluate(Evaluator &evaluator, return ActorIsolation::forUnspecified(); auto i = pbd->getPatternEntryIndexForVarDecl(var); - if (!pbd->isInitializerChecked(i)) - TypeChecker::typeCheckPatternBinding(pbd, i); dc = cast(pbd->getInitContext(i)); - initExpr = pbd->getInit(i); + initExpr = pbd->getCheckedAndContextualizedInit(i); enclosingIsolation = getActorIsolation(var); } else if (auto *param = dyn_cast(var)) { // If this parameter corresponds to a stored property for a diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp index d4bf26dba1aed..a9825b0e47e7d 100644 --- a/lib/Sema/TypeCheckDeclPrimary.cpp +++ b/lib/Sema/TypeCheckDeclPrimary.cpp @@ -2722,7 +2722,16 @@ class DeclChecker : public DeclVisitor { // Trigger a request that will complete typechecking for the // initializer. - (void)PBD->getCheckedAndContextualizedInit(i); + (void) PBD->getCheckedAndContextualizedInit(i); + + if (auto *var = PBD->getSingleVar()) { + if (var->hasAttachedPropertyWrapper()) + return; + } + + if (!PBD->getDeclContext()->isLocalContext()) { + (void) PBD->getInitializerIsolation(i); + } } } diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index 251d07439c0e9..a222632c54351 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -617,7 +617,6 @@ static void checkAndContextualizePatternBindingInit(PatternBindingDecl *binding, if (auto *initContext = binding->getInitContext(i)) { auto *init = binding->getInit(i); TypeChecker::contextualizeInitializer(initContext, init); - (void)binding->getInitializerIsolation(i); TypeChecker::checkInitializerEffects(initContext, init); } } diff --git a/test/Concurrency/sendable_checking_captures.swift b/test/Concurrency/sendable_checking_captures.swift new file mode 100644 index 0000000000000..3df34012421da --- /dev/null +++ b/test/Concurrency/sendable_checking_captures.swift @@ -0,0 +1,25 @@ +// RUN: %target-typecheck-verify-swift -swift-version 6 + +class NonSendable {} // expected-note 3{{class 'NonSendable' does not conform to the 'Sendable' protocol}} + +func callee(_: @Sendable () -> NonSendable) {} + +var testLocalCaptures: Int { + let ns = NonSendable() + + @Sendable func localFunc() -> NonSendable { + return ns // expected-error {{capture of 'ns' with non-sendable type 'NonSendable' in a `@Sendable` local function}} + } + + callee { return ns } // expected-error {{capture of 'ns' with non-sendable type 'NonSendable' in a `@Sendable` closure}} + + return 3 +} + +struct Bad { + var c: Int = { + let ns = NonSendable() + callee { return ns } // expected-error {{capture of 'ns' with non-sendable type 'NonSendable' in a `@Sendable` closure}} + return 3 + }() +} \ No newline at end of file diff --git a/test/Concurrency/sendable_checking_script_mode.swift b/test/Concurrency/sendable_checking_script_mode.swift deleted file mode 100644 index 1a6ae26825ee1..0000000000000 --- a/test/Concurrency/sendable_checking_script_mode.swift +++ /dev/null @@ -1,17 +0,0 @@ -// RUN: %target-typecheck-verify-swift -swift-version 6 - -class NonSendable {} // expected-note 2{{class 'NonSendable' does not conform to the 'Sendable' protocol}} - -var testLocalCaptures: Int { - let ns = NonSendable() - - @Sendable func localFunc() -> NonSendable { - return ns // expected-error {{capture of 'ns' with non-sendable type 'NonSendable' in a `@Sendable` local function}} - } - - let _: @Sendable () -> NonSendable = { - return ns // expected-error {{capture of 'ns' with non-sendable type 'NonSendable' in a `@Sendable` closure}} - } - - return 3 -} \ No newline at end of file