From 2d1c6f5f30e850bf0c411bcc1ee3edf16a27eb4f Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 8 Apr 2024 23:18:25 -0400 Subject: [PATCH 01/11] AST: Remove redundant call to getASTContext() --- lib/AST/ASTContext.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 74493186ea647..af73d0e907220 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -5875,7 +5875,7 @@ ASTContext::getOpenedExistentialSignature(Type type, GenericSignature parentSig) return found->second; auto genericParam = OpenedArchetypeType::getSelfInterfaceTypeFromContext( - canParentSig, type->getASTContext()) + canParentSig, *this) ->castTo(); Requirement requirement(RequirementKind::Conformance, genericParam, constraint); From 9ad5af6f2b4ec69290000b161aa4d9cfe6be23c4 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 9 Apr 2024 13:13:32 -0400 Subject: [PATCH 02/11] Serialization: Correctly set conforming type in readNormalProtocolConformance() Use the self interface type instead of the declared interface type, to get the right type for tuple conformances and the DistributedActor-as-Actor abstract conformance. --- lib/Serialization/Deserialization.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index f70be9efc28c9..976ca2461314e 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -987,7 +987,7 @@ ProtocolConformanceDeserializer::readNormalProtocolConformance( assert(!isa(dc->getModuleScopeContext()) && "should not have serialized a conformance from a clang module"); - Type conformingType = dc->getDeclaredInterfaceType(); + Type conformingType = dc->getSelfInterfaceType(); PrettyStackTraceType trace(ctx, "reading conformance for", conformingType); auto protoOrError = MF.getDeclChecked(protoID); From d8ff9670bfe0c53bb3eecffaf85df65a6c4ea2a7 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 9 Apr 2024 15:25:26 -0400 Subject: [PATCH 03/11] Sema: Use Requirement::getProtocolDecl() --- lib/Sema/CSSimplify.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 57d630445e162..ad420272dfcf8 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -10256,13 +10256,9 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName, if (req.getKind() != RequirementKind::Conformance) return false; - if (auto protocolTy = - req.getSecondType()->template getAs()) { - return req.getFirstType()->hasTypeVariable() && - protocolTy->getDecl()->isSpecificProtocol( - KnownProtocolKind::Sendable); - } - return false; + return (req.getFirstType()->hasTypeVariable() && + req.getProtocolDecl()->isSpecificProtocol( + KnownProtocolKind::Sendable)); })) { result.OverallResult = MemberLookupResult::Unsolved; return result; From 37344ab150ac2f887857561893a7269035c9a79a Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 9 Apr 2024 15:26:38 -0400 Subject: [PATCH 04/11] AST: Use Requirement::getProtocolDecl() --- lib/AST/ASTPrinter.cpp | 11 ++++------- lib/AST/Type.cpp | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index e692a86383e75..ae03dc01730f1 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -1683,14 +1683,11 @@ void PrintAST::printGenericSignature(GenericSignature genericSig, static void eraseInvertibleProtocolConformances( SmallVectorImpl &requirements) { llvm::erase_if(requirements, [&](Requirement req) { - if (req.getKind() == RequirementKind::Conformance) { - if (auto protoType = req.getSecondType()->getAs()) { - auto proto = protoType->getDecl(); - return proto->getInvertibleProtocolKind().has_value(); - } - } + if (req.getKind() != RequirementKind::Conformance) + return false; - return false; + return req.getProtocolDecl() + ->getInvertibleProtocolKind().has_value(); }); } diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index c28e4ef646fd4..a0f89a263d01c 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -954,7 +954,7 @@ Type TypeBase::stripConcurrency(bool recurse, bool dropGlobalActor) { // If it's a Sendable requirement, skip it. const auto &req = requirements[reqIdx]; if (req.getKind() == RequirementKind::Conformance && - req.getSecondType()->castTo()->getDecl() + req.getProtocolDecl() ->isSpecificProtocol(KnownProtocolKind::Sendable)) continue; From 6e14999ee2ea98f238fded2505052ec721839c78 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 12 Apr 2024 14:30:40 -0400 Subject: [PATCH 05/11] ASTDumper: Don't crash if capture info not computed yet --- lib/AST/ASTDumper.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp index e37496562f17e..e94b2e3df072e 100644 --- a/lib/AST/ASTDumper.cpp +++ b/lib/AST/ASTDumper.cpp @@ -1402,6 +1402,7 @@ namespace { printField(PD->getDefaultArgumentKind(), "default_arg"); } if (PD->hasDefaultExpr() && + PD->getDefaultArgumentCaptureInfo().hasBeenComputed() && !PD->getDefaultArgumentCaptureInfo().isTrivial()) { printFieldRaw([&](raw_ostream &OS) { PD->getDefaultArgumentCaptureInfo().print(OS); @@ -1488,7 +1489,8 @@ namespace { void printCommonAFD(AbstractFunctionDecl *D, const char *Type, StringRef Label) { printCommon(D, Type, Label, FuncColor); - if (!D->getCaptureInfo().isTrivial()) { + if (D->getCaptureInfo().hasBeenComputed() && + !D->getCaptureInfo().isTrivial()) { printFlagRaw([&](raw_ostream &OS) { D->getCaptureInfo().print(OS); }); @@ -2826,7 +2828,8 @@ class PrintExpr : public ExprVisitor, break; } - if (!E->getCaptureInfo().isTrivial()) { + if (E->getCaptureInfo().hasBeenComputed() && + !E->getCaptureInfo().isTrivial()) { printFieldRaw([&](raw_ostream &OS) { E->getCaptureInfo().print(OS); }, "", CapturesColor); From eb0472989b9a2c85efa4785ca3b91b7aaf65cb19 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 9 Apr 2024 15:19:55 -0400 Subject: [PATCH 06/11] Concurrency: Fix a formatting nit --- lib/Sema/TypeCheckConcurrency.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index 76d766810fd8b..b445fca99daff 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -4249,7 +4249,9 @@ void swift::checkFunctionActorIsolation(AbstractFunctionDecl *decl) { ActorIsolationChecker checker(decl); if (auto body = decl->getBody()) { body->walk(checker); - if(ctx.LangOpts.hasFeature(Feature::GroupActorErrors)){ checker.diagnoseIsolationErrors(); } + if (ctx.LangOpts.hasFeature(Feature::GroupActorErrors)) { + checker.diagnoseIsolationErrors(); + } } if (auto ctor = dyn_cast(decl)) { if (auto superInit = ctor->getSuperInitCall()) From 0c185c4c5a8d7987e6fd0904bdb561475921238c Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 12 Apr 2024 13:16:28 -0400 Subject: [PATCH 07/11] 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 b445fca99daff..1d928d8e045ee 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2625,6 +2625,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 e59a9d8648c7c85714b7f7af294ad913123a4266 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 12 Apr 2024 14:46:23 -0400 Subject: [PATCH 08/11] 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 1d928d8e045ee..31481c3c574f0 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2633,7 +2633,7 @@ namespace { } if (auto func = dyn_cast(decl)) { - if (func->isLocalContext()) { + if (func->getDeclContext()->isLocalContext()) { checkLocalCaptures(func); } From 3b29aad017e529378f3a0387f33a18d8a34bc3a5 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 12 Apr 2024 13:40:32 -0400 Subject: [PATCH 09/11] 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 ed43dec0d2373c0142861d246f95e6fba4a90fa2 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 12 Apr 2024 15:04:11 -0400 Subject: [PATCH 10/11] 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 31481c3c574f0..2ec7be37a5d75 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2624,6 +2624,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 5a393697244c636a4457073f3aff221c0c16744c Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 12 Apr 2024 16:39:28 -0400 Subject: [PATCH 11/11] 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 2ec7be37a5d75..fc78a38abc141 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -5391,11 +5391,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