From b5da8d54fd3e426098f302aa02421590e7e47380 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 31 Jul 2023 14:50:15 -0700 Subject: [PATCH 1/7] [Type refinement context] Don't query property wrappers just for range info Querying property wrappers involves semantic analysis that can cause cyclic references while building the type refinement context, and it's unnecessary: we need only know that these are custom attributes to incorporate their source ranges. Switch to the simpler/cheaper query. A small part of fixing the cyclic references in rdar://112079160. --- lib/Sema/TypeCheckAvailability.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 2c3e02df9b3ed..79abc3fa81e8d 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -679,13 +679,16 @@ class TypeRefinementContextBuilder : private ASTWalker { // For a variable declaration (without accessors) we use the range of the // containing pattern binding declaration to make sure that we include // any type annotation in the type refinement context range. We also - // need to include any attached property wrappers. + // need to include any custom attributes that were written on the + // declaration. if (auto *varDecl = dyn_cast(storageDecl)) { if (auto *PBD = varDecl->getParentPatternBinding()) Range = PBD->getSourceRange(); - for (auto *propertyWrapper : varDecl->getAttachedPropertyWrappers()) { - Range.widen(propertyWrapper->getRange()); + for (auto attr : varDecl->getOriginalAttrs()) { + if (auto customAttr = dyn_cast(attr)) { + Range.widen(customAttr->getRange()); + } } } @@ -696,7 +699,7 @@ class TypeRefinementContextBuilder : private ASTWalker { // locations and have callers of that method provide appropriate source // locations. SourceRange BracesRange = storageDecl->getBracesRange(); - if (storageDecl->hasParsedAccessors() && BracesRange.isValid()) { + if (BracesRange.isValid()) { Range.widen(BracesRange); } From a5fdee9726db118e1bfaf174e48a61216d8f8cb7 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 31 Jul 2023 22:43:01 -0700 Subject: [PATCH 2/7] [Type refinement context] Lazily expand TRCs for pattern bindings Eager expansion of type refinement contexts (TRCs) for variables within pattern binding declarations is causing cyclic references in some places involving macros. Make this expansion lazy, triggered by walking into these pattern binding declarations as part of (e.g.) availability queries. Another step toward fixing the cyclic references in rdar://112079160. --- include/swift/AST/TypeCheckRequests.h | 20 +++ include/swift/AST/TypeCheckerTypeIDZone.def | 3 + include/swift/AST/TypeRefinementContext.h | 3 + lib/AST/TypeRefinementContext.cpp | 18 +++ lib/Sema/TypeCheckAvailability.cpp | 164 +++++++++++++------- 5 files changed, 152 insertions(+), 56 deletions(-) diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index b278bd49cc55e..00695f1b496de 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -63,6 +63,7 @@ class TypeAliasDecl; class TypeLoc; class Witness; class TypeResolution; +class TypeRefinementContext; struct TypeWitnessAndDecl; class ValueDecl; enum class OpaqueReadOwnership: uint8_t; @@ -4387,6 +4388,25 @@ class InitAccessorReferencedVariablesRequest bool isCached() const { return true; } }; +/// Expand the children of the type refinement context for the given +/// declaration. +class ExpandChildTypeRefinementContextsRequest + : public SimpleRequest { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + bool evaluate(Evaluator &evaluator, Decl *decl, + TypeRefinementContext *parentTRC) const; + +public: + bool isCached() const { return true; } +}; + #define SWIFT_TYPEID_ZONE TypeChecker #define SWIFT_TYPEID_HEADER "swift/AST/TypeCheckerTypeIDZone.def" #include "swift/Basic/DefineTypeIDZone.h" diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index 8e697933619ca..ea024e9f1d2d4 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/include/swift/AST/TypeCheckerTypeIDZone.def @@ -500,3 +500,6 @@ SWIFT_REQUEST(TypeChecker, InitAccessorReferencedVariablesRequest, ArrayRef(DeclAttribute *, AccessorDecl *, ArrayRef), Cached, NoLocationInfo) +SWIFT_REQUEST(TypeChecker, ExpandChildTypeRefinementContextsRequest, + bool(Decl *, TypeRefinementContext *), + Cached, NoLocationInfo) diff --git a/include/swift/AST/TypeRefinementContext.h b/include/swift/AST/TypeRefinementContext.h index dbe492476b8b7..22e78af500955 100644 --- a/include/swift/AST/TypeRefinementContext.h +++ b/include/swift/AST/TypeRefinementContext.h @@ -297,6 +297,9 @@ class TypeRefinementContext : public ASTAllocated { static StringRef getReasonName(Reason R); }; +void simple_display(llvm::raw_ostream &out, + const TypeRefinementContext *trc); + } // end namespace swift #endif diff --git a/lib/AST/TypeRefinementContext.cpp b/lib/AST/TypeRefinementContext.cpp index 99e84e26b6c55..d349b1114971f 100644 --- a/lib/AST/TypeRefinementContext.cpp +++ b/lib/AST/TypeRefinementContext.cpp @@ -20,6 +20,7 @@ #include "swift/AST/Stmt.h" #include "swift/AST/Expr.h" #include "swift/AST/SourceFile.h" +#include "swift/AST/TypeCheckRequests.h" #include "swift/AST/TypeRefinementContext.h" #include "swift/Basic/SourceManager.h" @@ -192,6 +193,18 @@ TypeRefinementContext::findMostRefinedSubContext(SourceLoc Loc, !rangeContainsTokenLocWithGeneratedSource(SM, SrcRange, Loc)) return nullptr; + // If this context is for a declaration, ensure that we've expanded the + // children of the declaration. + if (Node.getReason() == Reason::Decl || + Node.getReason() == Reason::DeclImplicit) { + if (auto decl = Node.getAsDecl()) { + ASTContext &ctx = decl->getASTContext(); + (void)evaluateOrDefault( + ctx.evaluator, ExpandChildTypeRefinementContextsRequest{decl, this}, + false); + } + } + // For the moment, we perform a linear search here, but we can and should // do something more efficient. for (TypeRefinementContext *Child : Children) { @@ -411,3 +424,8 @@ StringRef TypeRefinementContext::getReasonName(Reason R) { llvm_unreachable("Unhandled Reason in switch."); } + +void swift::simple_display( + llvm::raw_ostream &out, const TypeRefinementContext *trc) { + out << "TRC @" << trc; +} diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 79abc3fa81e8d..acb3ce43a6a37 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -440,6 +440,8 @@ class TypeRefinementContextBuilder : private ASTWalker { return "building type refinement context for"; } + friend class swift::ExpandChildTypeRefinementContextsRequest; + public: TypeRefinementContextBuilder(TypeRefinementContext *TRC, ASTContext &Context) : Context(Context) { @@ -576,9 +578,10 @@ class TypeRefinementContextBuilder : private ASTWalker { // pattern. The context necessary for the pattern as a whole was already // introduced if necessary by the first var decl. if (auto *VD = dyn_cast(D)) { - if (auto *PBD = VD->getParentPatternBinding()) + if (auto *PBD = VD->getParentPatternBinding()) { if (VD != PBD->getAnchoringVarDecl(0)) return nullptr; + } } // Declarations with an explicit availability attribute always get a TRC. @@ -597,11 +600,16 @@ class TypeRefinementContextBuilder : private ASTWalker { // internal property in a public struct can be effectively less available // than the containing struct decl because the internal property will only // be accessed by code running at the deployment target or later. + // + // For declarations that have their child construction delayed, always + // create this implicit declaration context. It will be used to trigger + // lazy creation of the child TRCs. AvailabilityContext CurrentAvailability = getCurrentTRC()->getAvailabilityInfo(); AvailabilityContext EffectiveAvailability = getEffectiveAvailabilityForDeclSignature(D, CurrentAvailability); - if (CurrentAvailability.isSupersetOf(EffectiveAvailability)) + if (isa(D) || + CurrentAvailability.isSupersetOf(EffectiveAvailability)) return TypeRefinementContext::createForDeclImplicit( Context, D, getCurrentTRC(), EffectiveAvailability, refinementSourceRangeForDecl(D)); @@ -709,28 +717,79 @@ class TypeRefinementContextBuilder : private ASTWalker { return D->getSourceRange(); } + // Creates an implicit decl TRC specifying the deployment + // target for `range` in decl `D`. + TypeRefinementContext * + createImplicitDeclContextForDeploymentTarget(Decl *D, SourceRange range){ + AvailabilityContext Availability = + AvailabilityContext::forDeploymentTarget(Context); + Availability.intersectWith(getCurrentTRC()->getAvailabilityInfo()); + + return TypeRefinementContext::createForDeclImplicit( + Context, D, getCurrentTRC(), Availability, range); + } + + /// Build contexts for a VarDecl with the given initializer. + void buildContextsForPatternBindingDecl(PatternBindingDecl *pattern) { + // Build contexts for each of the pattern entries. + for (unsigned index : range(pattern->getNumPatternEntries())) { + auto var = pattern->getAnchoringVarDecl(index); + if (!var) + continue; + + // Var decls may have associated pattern binding decls or property wrappers + // with init expressions. Those expressions need to be constrained to the + // deployment target unless they are exposed to clients. + if (!var->hasInitialValue() || var->isInitExposedToClients()) + continue; + + auto *initExpr = pattern->getInit(index); + if (initExpr && !initExpr->isImplicit()) { + assert(initExpr->getSourceRange().isValid()); + + // Create a TRC for the init written in the source. The ASTWalker + // won't visit these expressions so instead of pushing these onto the + // stack we build them directly. + auto *initTRC = createImplicitDeclContextForDeploymentTarget( + var, initExpr->getSourceRange()); + TypeRefinementContextBuilder(initTRC, Context).build(initExpr); + } + } + + // Ideally any init expression would be returned by `getInit()` above. + // However, for property wrappers it doesn't get populated until + // typechecking completes (which is too late). Instead, we find the + // the property wrapper attribute and use its source range to create a + // TRC for the initializer expression. + // + // FIXME: Since we don't have an expression here, we can't build out its + // TRC. If the Expr that will eventually be created contains a closure + // expression, then it might have AST nodes that need to be refined. For + // example, property wrapper initializers that takes block arguments + // are not handled correctly because of this (rdar://77841331). + if (auto firstVar = pattern->getAnchoringVarDecl(0)) { + if (firstVar->hasInitialValue() && !firstVar->isInitExposedToClients()) { + for (auto *wrapper : firstVar->getAttachedPropertyWrappers()) { + createImplicitDeclContextForDeploymentTarget( + firstVar, wrapper->getRange()); + } + } + } + } + void buildContextsForBodyOfDecl(Decl *D) { // Are we already constrained by the deployment target? If not, adding // new contexts won't change availability. if (isCurrentTRCContainedByDeploymentTarget()) return; - // A lambda that creates an implicit decl TRC specifying the deployment - // target for `range` in decl `D`. - auto createContext = [this](Decl *D, SourceRange range) { - AvailabilityContext Availability = - AvailabilityContext::forDeploymentTarget(Context); - Availability.intersectWith(getCurrentTRC()->getAvailabilityInfo()); - - return TypeRefinementContext::createForDeclImplicit( - Context, D, getCurrentTRC(), Availability, range); - }; - // Top level code always uses the deployment target. if (auto tlcd = dyn_cast(D)) { if (auto bodyStmt = tlcd->getBody()) { - pushDeclBodyContext(createContext(tlcd, tlcd->getSourceRange()), tlcd, - bodyStmt); + pushDeclBodyContext( + createImplicitDeclContextForDeploymentTarget( + tlcd, tlcd->getSourceRange()), + tlcd, bodyStmt); } return; } @@ -741,51 +800,14 @@ class TypeRefinementContextBuilder : private ASTWalker { if (!afd->isImplicit() && afd->getResilienceExpansion() != ResilienceExpansion::Minimal) { if (auto body = afd->getBody(/*canSynthesize*/ false)) { - pushDeclBodyContext(createContext(afd, afd->getBodySourceRange()), - afd, body); + pushDeclBodyContext( + createImplicitDeclContextForDeploymentTarget( + afd, afd->getBodySourceRange()), + afd, body); } } return; } - - // Var decls may have associated pattern binding decls or property wrappers - // with init expressions. Those expressions need to be constrained to the - // deployment target unless they are exposed to clients. - if (auto vd = dyn_cast(D)) { - if (!vd->hasInitialValue() || vd->isInitExposedToClients()) - return; - - if (auto *pbd = vd->getParentPatternBinding()) { - int idx = pbd->getPatternEntryIndexForVarDecl(vd); - auto *initExpr = pbd->getInit(idx); - if (initExpr && !initExpr->isImplicit()) { - assert(initExpr->getSourceRange().isValid()); - - // Create a TRC for the init written in the source. The ASTWalker - // won't visit these expressions so instead of pushing these onto the - // stack we build them directly. - auto *initTRC = createContext(vd, initExpr->getSourceRange()); - TypeRefinementContextBuilder(initTRC, Context).build(initExpr); - } - - // Ideally any init expression would be returned by `getInit()` above. - // However, for property wrappers it doesn't get populated until - // typechecking completes (which is too late). Instead, we find the - // the property wrapper attribute and use its source range to create a - // TRC for the initializer expression. - // - // FIXME: Since we don't have an expression here, we can't build out its - // TRC. If the Expr that will eventually be created contains a closure - // expression, then it might have AST nodes that need to be refined. For - // example, property wrapper initializers that takes block arguments - // are not handled correctly because of this (rdar://77841331). - for (auto *wrapper : vd->getAttachedPropertyWrappers()) { - createContext(vd, wrapper->getRange()); - } - } - - return; - } } PreWalkResult walkToStmtPre(Stmt *S) override { @@ -1277,6 +1299,36 @@ TypeChecker::getOrBuildTypeRefinementContext(SourceFile *SF) { return TRC; } +bool ExpandChildTypeRefinementContextsRequest::evaluate( + Evaluator &evaluator, Decl *decl, TypeRefinementContext *parentTRC +) const { + // If the parent TRC is already contained by the deployment target, there's + // nothing more to do. + ASTContext &ctx = decl->getASTContext(); + if (computeContainedByDeploymentTarget(parentTRC, ctx)) + return false; + + // Variables can have children corresponding to property wrappers and + // the initial values provided in each pattern binding entry. + if (auto var = dyn_cast(decl)) { + if (auto *pattern = var->getParentPatternBinding()) { + // Only do this for the first variable in the pattern binding declaration. + auto anchorVar = pattern->getAnchoringVarDecl(0); + if (anchorVar != var) { + return evaluateOrDefault( + evaluator, + ExpandChildTypeRefinementContextsRequest{anchorVar, parentTRC}, + false); + } + + TypeRefinementContextBuilder builder(parentTRC, ctx); + builder.buildContextsForPatternBindingDecl(pattern); + } + } + + return false; +} + AvailabilityContext TypeChecker::overApproximateAvailabilityAtLocation(SourceLoc loc, const DeclContext *DC, From 355ce93b210a146a511034194fd834d4d3249527 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 1 Aug 2023 09:49:47 -0700 Subject: [PATCH 3/7] Make `VarDecl::isLayoutExposedToClients` check property wrappers more lazily The check for "has property wrappers" as part of determining whether the layout of a variable is exposed to clients can trigger reference cycles. Push this check later, which eliminates these cycles for types that aren't frozen/fixed-layout. This is a hack, not a real fix, but it eliminates the cyclic references observed in rdar://112079160. --- lib/AST/Decl.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index bb2312dab20a4..2e3202ee52167 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -2079,19 +2079,23 @@ bool VarDecl::isLayoutExposedToClients() const { if (!parent) return false; if (isStatic()) return false; - if (!hasStorage() && - !getAttrs().hasAttribute() && - !hasAttachedPropertyWrapper()) { - return false; - } auto nominalAccess = parent->getFormalAccessScope(/*useDC=*/nullptr, /*treatUsableFromInlineAsPublic=*/true); if (!nominalAccess.isPublic()) return false; - return (parent->getAttrs().hasAttribute() || - parent->getAttrs().hasAttribute()); + if (!parent->getAttrs().hasAttribute() && + !parent->getAttrs().hasAttribute()) + return false; + + if (!hasStorage() && + !getAttrs().hasAttribute() && + !hasAttachedPropertyWrapper()) { + return false; + } + + return true; } /// Check whether the given type representation will be From d1224fb496dbd40220678724ec466efa244ef11e Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 1 Aug 2023 14:11:32 -0700 Subject: [PATCH 4/7] [Type refinement context] Avoid creating implicit contexts with bad ranges --- lib/Sema/TypeCheckAvailability.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index acb3ce43a6a37..871e8da4b60ef 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -608,7 +608,7 @@ class TypeRefinementContextBuilder : private ASTWalker { getCurrentTRC()->getAvailabilityInfo(); AvailabilityContext EffectiveAvailability = getEffectiveAvailabilityForDeclSignature(D, CurrentAvailability); - if (isa(D) || + if ((isa(D) && refinementSourceRangeForDecl(D).isValid()) || CurrentAvailability.isSupersetOf(EffectiveAvailability)) return TypeRefinementContext::createForDeclImplicit( Context, D, getCurrentTRC(), EffectiveAvailability, From 8385f38ece015272dd29d1e592a0fb3c33916588 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 1 Aug 2023 16:10:04 -0700 Subject: [PATCH 5/7] Add test case involving circular references with `@Observable` Add a test case for Observable types that are extended from other source files. Prior to the recent changes to make `TypeRefinementContext` more lazy, this would trigger circular references through the `TypeRefinementContextBuilder`. Finishes rdar://112079160. --- .../Observation/Inputs/ObservableClass.swift | 7 +++++++ .../ObservableAvailabilityCycle.swift | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 test/stdlib/Observation/Inputs/ObservableClass.swift create mode 100644 test/stdlib/Observation/ObservableAvailabilityCycle.swift diff --git a/test/stdlib/Observation/Inputs/ObservableClass.swift b/test/stdlib/Observation/Inputs/ObservableClass.swift new file mode 100644 index 0000000000000..16819bdada192 --- /dev/null +++ b/test/stdlib/Observation/Inputs/ObservableClass.swift @@ -0,0 +1,7 @@ +import Foundation +import Observation + +@available(SwiftStdlib 5.9, *) +@Observable final public class ObservableClass { + public var state: State = .unused +} diff --git a/test/stdlib/Observation/ObservableAvailabilityCycle.swift b/test/stdlib/Observation/ObservableAvailabilityCycle.swift new file mode 100644 index 0000000000000..7343740f2b259 --- /dev/null +++ b/test/stdlib/Observation/ObservableAvailabilityCycle.swift @@ -0,0 +1,19 @@ +// REQUIRES: swift_swift_parser + +// RUN: %target-swift-frontend -typecheck -parse-as-library -enable-experimental-feature InitAccessors -external-plugin-path %swift-host-lib-dir/plugins#%swift-plugin-server -primary-file %s %S/Inputs/ObservableClass.swift + +// RUN: %target-swift-frontend -typecheck -parse-as-library -enable-experimental-feature InitAccessors -external-plugin-path %swift-host-lib-dir/plugins#%swift-plugin-server %s -primary-file %S/Inputs/ObservableClass.swift + +// REQUIRES: observation +// REQUIRES: concurrency +// REQUIRES: objc_interop +// UNSUPPORTED: use_os_stdlib +// UNSUPPORTED: back_deployment_runtime + +@available(SwiftStdlib 5.9, *) +extension ObservableClass { + @frozen public enum State: Sendable { + case unused + case used + } +} From f32cae9215ebea745746e84a8adcac3274b77c83 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 2 Aug 2023 07:38:19 -0700 Subject: [PATCH 6/7] Drop unnecessary "parent context" state from TypeRefinementContextBuilder This state is a holdover from when accessors we stored "alongside" their variable declarations, rather than contained within them. That's no longer the case, so we don't need to track this information any more. --- lib/Sema/TypeCheckAvailability.cpp | 67 ------------------------------ 1 file changed, 67 deletions(-) diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 871e8da4b60ef..384d1db22fb70 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -388,20 +388,6 @@ class TypeRefinementContextBuilder : private ASTWalker { }; std::vector DeclBodyContextStack; - /// A mapping from abstract storage declarations with accessors to - /// to the type refinement contexts for those declarations. We refer to - /// this map to determine the appropriate parent TRC to use when - /// walking the accessor function. - llvm::DenseMap - StorageContexts; - - /// A mapping from pattern binding storage declarations to the type refinement - /// contexts for those declarations. We refer to this map to determine the - /// appropriate parent TRC to use when walking a var decl that belongs to a - /// pattern containing multiple vars. - llvm::DenseMap - PatternBindingContexts; - TypeRefinementContext *getCurrentTRC() { return ContextStack.back().TRC; } @@ -482,19 +468,9 @@ class TypeRefinementContextBuilder : private ASTWalker { PreWalkAction walkToDeclPre(Decl *D) override { PrettyStackTraceDecl trace(stackTraceAction(), D); - // Adds in a parent TRC for decls which are syntactically nested but are not - // represented that way in the AST. (Particularly, AbstractStorageDecl - // parents for AccessorDecl children.) - if (auto ParentTRC = getEffectiveParentContextForDecl(D)) { - pushContext(ParentTRC, D); - } - // Adds in a TRC that covers the entire declaration. if (auto DeclTRC = getNewContextForSignatureOfDecl(D)) { pushContext(DeclTRC, D); - - // Possibly use this as an effective parent context later. - recordEffectiveParentContext(D, DeclTRC); } // Create TRCs that cover only the body of the declaration. @@ -515,49 +491,6 @@ class TypeRefinementContextBuilder : private ASTWalker { return Action::Continue(); } - TypeRefinementContext *getEffectiveParentContextForDecl(Decl *D) { - // FIXME: Can we assert that we won't walk parent decls later that should - // have been returned here? - if (auto *accessor = dyn_cast(D)) { - // Use TRC of the storage rather the current TRC when walking this - // function. - auto it = StorageContexts.find(accessor->getStorage()); - if (it != StorageContexts.end()) { - return it->second; - } - } else if (auto *VD = dyn_cast(D)) { - // Use the TRC of the pattern binding decl as the parent for var decls. - if (auto *PBD = VD->getParentPatternBinding()) { - auto it = PatternBindingContexts.find(PBD); - if (it != PatternBindingContexts.end()) { - return it->second; - } - } - } - - return nullptr; - } - - /// If necessary, records a TRC so it can be returned by subsequent calls to - /// `getEffectiveParentContextForDecl()`. - void recordEffectiveParentContext(Decl *D, TypeRefinementContext *NewTRC) { - if (auto *StorageDecl = dyn_cast(D)) { - // Stash the TRC for the storage declaration to use as the parent of - // accessor decls later. - if (StorageDecl->hasParsedAccessors()) - StorageContexts[StorageDecl] = NewTRC; - } - - if (auto *VD = dyn_cast(D)) { - // Stash the TRC for the var decl if its parent pattern binding decl has - // more than one entry so that the sibling var decls can reuse it. - if (auto *PBD = VD->getParentPatternBinding()) { - if (PBD->getNumPatternEntries() > 1) - PatternBindingContexts[PBD] = NewTRC; - } - } - } - /// Returns a new context to be introduced for the declaration, or nullptr /// if no new context should be introduced. TypeRefinementContext *getNewContextForSignatureOfDecl(Decl *D) { From 29f102284505cd55d6ea57ae869154d708089906 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 2 Aug 2023 10:30:59 -0700 Subject: [PATCH 7/7] Establish type refinement contexts for pattern binding decls directly The type refinement context builder had a bunch of logic to try to model type refinement contexts for the first variable declaration that shows up within a pattern binding declaration. Instead, model this more syntactically by creating a type refinement context for the pattern binding declaration itself. This both addresses a regression in the handling of `if #available` within a closure that's part of an initializer, and fixes a bug in the same area where similar code has explicit availability annotations. --- include/swift/AST/Availability.h | 7 ++ lib/AST/Availability.cpp | 30 ++++++++ lib/AST/TypeRefinementContext.cpp | 4 + lib/Sema/TypeCheckAvailability.cpp | 106 +++++++------------------- test/Sema/availability_versions.swift | 22 ++++++ 5 files changed, 91 insertions(+), 78 deletions(-) diff --git a/include/swift/AST/Availability.h b/include/swift/AST/Availability.h index 8c05435a999c8..a6d8f80091b1b 100644 --- a/include/swift/AST/Availability.h +++ b/include/swift/AST/Availability.h @@ -380,6 +380,13 @@ class AvailabilityInference { }; +/// Given a declaration upon which an availability attribute would appear in +/// concrete syntax, return a declaration to which the parser +/// actually attaches the attribute in the abstract syntax tree. We use this +/// function to determine whether the concrete syntax already has an +/// availability attribute. +const Decl *abstractSyntaxDeclForAvailableAttribute(const Decl *D); + } // end namespace swift #endif diff --git a/lib/AST/Availability.cpp b/lib/AST/Availability.cpp index b000276714180..a07c70cec04df 100644 --- a/lib/AST/Availability.cpp +++ b/lib/AST/Availability.cpp @@ -224,6 +224,8 @@ AvailabilityInference::attrForAnnotatedAvailableRange(const Decl *D, ASTContext &Ctx) { const AvailableAttr *bestAvailAttr = nullptr; + D = abstractSyntaxDeclForAvailableAttribute(D); + for (auto Attr : D->getAttrs()) { auto *AvailAttr = dyn_cast(Attr); if (AvailAttr == nullptr || !AvailAttr->Introduced.has_value() || @@ -744,3 +746,31 @@ ASTContext::getSwift5PlusAvailability(llvm::VersionTuple swiftVersion) { Twine("Missing call to getSwiftXYAvailability for Swift ") + swiftVersion.getAsString()); } + +const Decl * +swift::abstractSyntaxDeclForAvailableAttribute(const Decl *ConcreteSyntaxDecl) { + // This function needs to be kept in sync with its counterpart, + // concreteSyntaxDeclForAvailableAttribute(). + + if (auto *PBD = dyn_cast(ConcreteSyntaxDecl)) { + // Existing @available attributes in the AST are attached to VarDecls + // rather than PatternBindingDecls, so we return the first VarDecl for + // the pattern binding declaration. + // This is safe, even though there may be multiple VarDecls, because + // all parsed attribute that appear in the concrete syntax upon on the + // PatternBindingDecl are added to all of the VarDecls for the pattern + // binding. + for (auto index : range(PBD->getNumPatternEntries())) { + if (auto VD = PBD->getAnchoringVarDecl(index)) + return VD; + } + } else if (auto *ECD = dyn_cast(ConcreteSyntaxDecl)) { + // Similar to the PatternBindingDecl case above, we return the + // first EnumElementDecl. + if (auto *Elem = ECD->getFirstElement()) { + return Elem; + } + } + + return ConcreteSyntaxDecl; +} diff --git a/lib/AST/TypeRefinementContext.cpp b/lib/AST/TypeRefinementContext.cpp index d349b1114971f..cde3ebff744b2 100644 --- a/lib/AST/TypeRefinementContext.cpp +++ b/lib/AST/TypeRefinementContext.cpp @@ -367,6 +367,10 @@ void TypeRefinementContext::print(raw_ostream &OS, SourceManager &SrcMgr, OS << "extension." << ED->getExtendedType().getString(); } else if (isa(D)) { OS << ""; + } else if (auto PBD = dyn_cast(D)) { + if (auto VD = PBD->getAnchoringVarDecl(0)) { + OS << VD->getName(); + } } } diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 384d1db22fb70..9b2d739a5723b 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -308,6 +308,8 @@ ExportContext::getExportabilityReason() const { /// on the target platform. static const AvailableAttr *getActiveAvailableAttribute(const Decl *D, ASTContext &AC) { + D = abstractSyntaxDeclForAvailableAttribute(D); + for (auto Attr : D->getAttrs()) if (auto AvAttr = dyn_cast(Attr)) { if (!AvAttr->isInvalid() && AvAttr->isActivePlatform(AC)) { @@ -494,7 +496,10 @@ class TypeRefinementContextBuilder : private ASTWalker { /// Returns a new context to be introduced for the declaration, or nullptr /// if no new context should be introduced. TypeRefinementContext *getNewContextForSignatureOfDecl(Decl *D) { - if (!isa(D) && !isa(D) && !isa(D)) + if (!isa(D) && + !isa(D) && + !isa(D) && + !isa(D)) return nullptr; // Only introduce for an AbstractStorageDecl if it is not local. We @@ -503,20 +508,17 @@ class TypeRefinementContextBuilder : private ASTWalker { if (isa(D) && D->getDeclContext()->isLocalContext()) return nullptr; + // Don't introduce for variable declarations that have a parent pattern + // binding; all of the relevant information is on the pattern binding. + if (auto var = dyn_cast(D)) { + if (var->getParentPatternBinding()) + return nullptr; + } + // Ignore implicit declarations (mainly skips over `DeferStmt` functions). if (D->isImplicit()) return nullptr; - // Skip introducing additional contexts for var decls past the first in a - // pattern. The context necessary for the pattern as a whole was already - // introduced if necessary by the first var decl. - if (auto *VD = dyn_cast(D)) { - if (auto *PBD = VD->getParentPatternBinding()) { - if (VD != PBD->getAnchoringVarDecl(0)) - return nullptr; - } - } - // Declarations with an explicit availability attribute always get a TRC. if (hasActiveAvailableAttribute(D, Context)) { AvailabilityContext DeclaredAvailability = @@ -541,7 +543,8 @@ class TypeRefinementContextBuilder : private ASTWalker { getCurrentTRC()->getAvailabilityInfo(); AvailabilityContext EffectiveAvailability = getEffectiveAvailabilityForDeclSignature(D, CurrentAvailability); - if ((isa(D) && refinementSourceRangeForDecl(D).isValid()) || + if ((isa(D) && + refinementSourceRangeForDecl(D).isValid()) || CurrentAvailability.isSupersetOf(EffectiveAvailability)) return TypeRefinementContext::createForDeclImplicit( Context, D, getCurrentTRC(), EffectiveAvailability, @@ -617,22 +620,6 @@ class TypeRefinementContextBuilder : private ASTWalker { // the bodies of its accessors. SourceRange Range = storageDecl->getSourceRange(); - // For a variable declaration (without accessors) we use the range of the - // containing pattern binding declaration to make sure that we include - // any type annotation in the type refinement context range. We also - // need to include any custom attributes that were written on the - // declaration. - if (auto *varDecl = dyn_cast(storageDecl)) { - if (auto *PBD = varDecl->getParentPatternBinding()) - Range = PBD->getSourceRange(); - - for (auto attr : varDecl->getOriginalAttrs()) { - if (auto customAttr = dyn_cast(attr)) { - Range.widen(customAttr->getRange()); - } - } - } - // HACK: For synthesized trivial accessors we may have not a valid // location for the end of the braces, so in that case we will fall back // to using the range for the storage declaration. The right fix here is @@ -646,7 +633,13 @@ class TypeRefinementContextBuilder : private ASTWalker { return Range; } - + + // For pattern binding declarations, include the attributes in the source + // range so that we're sure to cover any property wrappers. + if (auto patternBinding = dyn_cast(D)) { + return D->getSourceRangeIncludingAttrs(); + } + return D->getSourceRange(); } @@ -662,7 +655,7 @@ class TypeRefinementContextBuilder : private ASTWalker { Context, D, getCurrentTRC(), Availability, range); } - /// Build contexts for a VarDecl with the given initializer. + /// Build contexts for a pattern binding declaration. void buildContextsForPatternBindingDecl(PatternBindingDecl *pattern) { // Build contexts for each of the pattern entries. for (unsigned index : range(pattern->getNumPatternEntries())) { @@ -1241,22 +1234,11 @@ bool ExpandChildTypeRefinementContextsRequest::evaluate( if (computeContainedByDeploymentTarget(parentTRC, ctx)) return false; - // Variables can have children corresponding to property wrappers and - // the initial values provided in each pattern binding entry. - if (auto var = dyn_cast(decl)) { - if (auto *pattern = var->getParentPatternBinding()) { - // Only do this for the first variable in the pattern binding declaration. - auto anchorVar = pattern->getAnchoringVarDecl(0); - if (anchorVar != var) { - return evaluateOrDefault( - evaluator, - ExpandChildTypeRefinementContextsRequest{anchorVar, parentTRC}, - false); - } - - TypeRefinementContextBuilder builder(parentTRC, ctx); - builder.buildContextsForPatternBindingDecl(pattern); - } + // Pattern binding declarations can have children corresponding to property + // wrappers and the initial values provided in each pattern binding entry. + if (auto *binding = dyn_cast(decl)) { + TypeRefinementContextBuilder builder(parentTRC, ctx); + builder.buildContextsForPatternBindingDecl(binding); } return false; @@ -1596,38 +1578,6 @@ concreteSyntaxDeclForAvailableAttribute(const Decl *AbstractSyntaxDecl) { return AbstractSyntaxDecl; } -/// Given a declaration upon which an availability attribute would appear in -/// concrete syntax, return a declaration to which the parser -/// actually attaches the attribute in the abstract syntax tree. We use this -/// function to determine whether the concrete syntax already has an -/// availability attribute. -static const Decl * -abstractSyntaxDeclForAvailableAttribute(const Decl *ConcreteSyntaxDecl) { - // This function needs to be kept in sync with its counterpart, - // concreteSyntaxDeclForAvailableAttribute(). - - if (auto *PBD = dyn_cast(ConcreteSyntaxDecl)) { - // Existing @available attributes in the AST are attached to VarDecls - // rather than PatternBindingDecls, so we return the first VarDecl for - // the pattern binding declaration. - // This is safe, even though there may be multiple VarDecls, because - // all parsed attribute that appear in the concrete syntax upon on the - // PatternBindingDecl are added to all of the VarDecls for the pattern - // binding. - if (PBD->getNumPatternEntries() != 0) { - return PBD->getAnchoringVarDecl(0); - } - } else if (auto *ECD = dyn_cast(ConcreteSyntaxDecl)) { - // Similar to the PatternBindingDecl case above, we return the - // first EnumElementDecl. - if (auto *Elem = ECD->getFirstElement()) { - return Elem; - } - } - - return ConcreteSyntaxDecl; -} - /// Given a declaration, return a better related declaration for which /// to suggest an @available fixit, or the original declaration /// if no such related declaration exists. diff --git a/test/Sema/availability_versions.swift b/test/Sema/availability_versions.swift index 7618d2943bf86..da27f7b9b695e 100644 --- a/test/Sema/availability_versions.swift +++ b/test/Sema/availability_versions.swift @@ -1741,3 +1741,25 @@ func useHasUnavailableExtension(_ s: HasUnavailableExtension) { s.inheritsUnavailable() // expected-error {{'inheritsUnavailable()' is unavailable in macOS}} s.moreAvailableButStillUnavailable() // expected-error {{'moreAvailableButStillUnavailable()' is unavailable in macOS}} } + +@available(macOS 10.15, *) +func f() -> Int { 17 } + +class StoredPropertiesWithAvailabilityInClosures { + private static let value: Int = { + if #available(macOS 10.15, *) { + return f() + } + + return 0 + }() + + @available(macOS 10.14, *) + private static let otherValue: Int = { + if #available(macOS 10.15, *) { + return f() + } + + return 0 + }() +}