From 824ecdd0aa9da889c13a3266b6f31d8194062707 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 6 Oct 2020 00:54:21 -0400 Subject: [PATCH 1/5] ASTScope: Fix AttachedPropertyWrapperScope source range The source range needs to contain the argument expression. --- include/swift/AST/ASTScope.h | 13 +------------ lib/AST/ASTScopeSourceRange.cpp | 2 +- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/include/swift/AST/ASTScope.h b/include/swift/AST/ASTScope.h index c6f6c2a46cfdc..779e486185689 100644 --- a/include/swift/AST/ASTScope.h +++ b/include/swift/AST/ASTScope.h @@ -966,19 +966,8 @@ class AttachedPropertyWrapperScope final : public ASTScopeImpl { CustomAttr *attr; VarDecl *decl; - /// Because we have to avoid request cycles, we approximate the test for an - /// AttachedPropertyWrapper with one based on source location. We might get - /// false positives, that that doesn't hurt anything. However, the result of - /// the conservative source range computation doesn't seem to be stable. So - /// keep the original here, and use it for source range queries. - const SourceRange sourceRangeWhenCreated; - AttachedPropertyWrapperScope(CustomAttr *attr, VarDecl *decl) - : attr(attr), decl(decl), - sourceRangeWhenCreated(attr->getTypeRepr()->getSourceRange()) { - ASTScopeAssert(sourceRangeWhenCreated.isValid(), - "VarDecls must have ranges to be looked-up"); - } + : attr(attr), decl(decl) {} virtual ~AttachedPropertyWrapperScope() {} protected: diff --git a/lib/AST/ASTScopeSourceRange.cpp b/lib/AST/ASTScopeSourceRange.cpp index 1d93bdd762b84..89acd79aacddb 100644 --- a/lib/AST/ASTScopeSourceRange.cpp +++ b/lib/AST/ASTScopeSourceRange.cpp @@ -448,7 +448,7 @@ SourceRange ClosureParametersScope::getSourceRangeOfThisASTNode( SourceRange AttachedPropertyWrapperScope::getSourceRangeOfThisASTNode( const bool omitAssertions) const { - return sourceRangeWhenCreated; + return attr->getRange(); } SourceRange GuardStmtScope::getSourceRangeOfThisASTNode( From 66873b94c9c4adada9cdef3fa4866325f1050b35 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 6 Oct 2020 00:56:31 -0400 Subject: [PATCH 2/5] ASTScope: Fix SubscriptDeclScope source range The source range needs to contain any attributes on the subscript declaration, since those might create child scopes (@_specialize, property wrappers, etc). --- lib/AST/ASTScopeSourceRange.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/AST/ASTScopeSourceRange.cpp b/lib/AST/ASTScopeSourceRange.cpp index 89acd79aacddb..c377febaef6e7 100644 --- a/lib/AST/ASTScopeSourceRange.cpp +++ b/lib/AST/ASTScopeSourceRange.cpp @@ -226,7 +226,7 @@ SourceRange TopLevelCodeScope::getSourceRangeOfThisASTNode( SourceRange SubscriptDeclScope::getSourceRangeOfThisASTNode( const bool omitAssertions) const { - return decl->getSourceRange(); + return decl->getSourceRangeIncludingAttrs(); } SourceRange From 2e67c135fd286c9b942c79524aad739720f1cf3c Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 2 Oct 2020 02:43:21 -0400 Subject: [PATCH 3/5] ASTScope: Rework ConditionalClauseScopes The top-level scope for a conditional clause with a pattern is now ConditionalClausePatternUseScope, which introduces the pattern's bindings. Since the patterns are not visible in their own initializer, a new ConditionalClauseInitializerScope is used for the initializer. While it is nested inside of the ConditionalClausePatternUseScope, it's lookup parent skips one level, giving us the desired behavior. --- include/swift/AST/ASTScope.h | 80 +++++------- lib/AST/ASTScope.cpp | 13 +- lib/AST/ASTScopeCreation.cpp | 117 ++++++++++++------ lib/AST/ASTScopeLookup.cpp | 28 +++-- lib/AST/ASTScopePrinting.cpp | 7 +- lib/AST/ASTScopeSourceRange.cpp | 33 ++--- .../NameLookup/scope_map-astscopelookup.swift | 2 +- test/NameLookup/scope_map_top_level.swift | 12 +- 8 files changed, 146 insertions(+), 146 deletions(-) diff --git a/include/swift/AST/ASTScope.h b/include/swift/AST/ASTScope.h index 779e486185689..222b66a2167a8 100644 --- a/include/swift/AST/ASTScope.h +++ b/include/swift/AST/ASTScope.h @@ -1082,45 +1082,27 @@ class PatternEntryInitializerScope final : public AbstractPatternEntryScope { bool lookupLocalsOrMembers(DeclConsumer) const override; }; -/// The scope introduced by a conditional clause in an if/guard/while -/// statement. -/// Since there may be more than one "let foo = ..." in (e.g.) an "if", -/// we allocate a matrushka of these. -class ConditionalClauseScope final : public ASTScopeImpl { +/// The scope introduced by a conditional clause initializer in an +/// if/while/guard statement. +class ConditionalClauseInitializerScope final : public ASTScopeImpl { public: - LabeledConditionalStmt *const stmt; - const unsigned index; - const SourceLoc endLoc; // cannot get it from the stmt - - ConditionalClauseScope(LabeledConditionalStmt *stmt, unsigned index, - SourceLoc endLoc) - : stmt(stmt), index(index), endLoc(endLoc) {} - - virtual ~ConditionalClauseScope() {} - -protected: - ASTScopeImpl *expandSpecifically(ScopeCreator &scopeCreator) override; + Expr *const initializer; + const SourceRange bodyRange; -private: - AnnotatedInsertionPoint - expandAScopeThatCreatesANewInsertionPoint(ScopeCreator &); - -public: - std::string getClassName() const override; - -protected: - void printSpecifics(llvm::raw_ostream &out) const override; + ConditionalClauseInitializerScope(Expr *initializer) + : initializer(initializer) {} -public: + virtual ~ConditionalClauseInitializerScope() {} SourceRange getSourceRangeOfThisASTNode(bool omitAssertions = false) const override; + std::string getClassName() const override; private: - ArrayRef getCond() const; - const StmtConditionElement &getStmtConditionElement() const; + void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &); protected: - bool isLabeledStmtLookupTerminator() const override; + ASTScopeImpl *expandSpecifically(ScopeCreator &scopeCreator) override; + NullablePtr getLookupParent() const override; }; /// If, while, & guard statements all start with a conditional clause, then some @@ -1128,17 +1110,21 @@ class ConditionalClauseScope final : public ASTScopeImpl { /// the normal lookup rule to pass the lookup scope into the deepest conditional /// clause. class ConditionalClausePatternUseScope final : public ASTScopeImpl { - Pattern *const pattern; - const SourceLoc startLoc; + StmtConditionElement sec; + SourceLoc endLoc; public: - ConditionalClausePatternUseScope(Pattern *pattern, SourceLoc startLoc) - : pattern(pattern), startLoc(startLoc) {} + ConditionalClausePatternUseScope(StmtConditionElement sec, SourceLoc endLoc) + : sec(sec), endLoc(endLoc) {} SourceRange getSourceRangeOfThisASTNode(bool omitAssertions = false) const override; std::string getClassName() const override; +private: + AnnotatedInsertionPoint + expandAScopeThatCreatesANewInsertionPoint(ScopeCreator &); + protected: ASTScopeImpl *expandSpecifically(ScopeCreator &) override; bool lookupLocalsOrMembers(DeclConsumer) const override; @@ -1350,7 +1336,7 @@ class LabeledConditionalStmtScope : public AbstractStmtScope { protected: /// Return the lookupParent required to search these. ASTScopeImpl *createNestedConditionalClauseScopes(ScopeCreator &, - const Stmt *afterConds); + SourceLoc); }; class IfStmtScope final : public LabeledConditionalStmtScope { @@ -1408,28 +1394,24 @@ class GuardStmtScope final : public LabeledConditionalStmtScope { getSourceRangeOfThisASTNode(bool omitAssertions = false) const override; }; -/// A scope after a guard statement that follows lookups into the conditions -/// Also for: -/// The insertion point of the last statement of an active clause in an #if -/// must be the lookup parent -/// of any following scopes. But the active clause may not be the last clause. -/// In short, this is another case where the lookup parent cannot follow the same -/// nesting as the source order. IfConfigUseScope implements this twist. It -/// follows the IfConfig, wraps all subsequent scopes, and redirects the lookup. -class LookupParentDiversionScope final : public ASTScopeImpl { +/// A scope for the body of a guard statement. Lookups from the body must +/// skip the parent scopes for introducing pattern bindings, since they're +/// not visible in the guard body, only after the body ends. +class GuardStmtBodyScope final : public ASTScopeImpl { public: ASTScopeImpl *const lookupParent; - const SourceLoc startLoc; - const SourceLoc endLoc; + BraceStmt *const body; - LookupParentDiversionScope(ASTScopeImpl *lookupParent, - SourceLoc startLoc, SourceLoc endLoc) - : lookupParent(lookupParent), startLoc(startLoc), endLoc(endLoc) {} + GuardStmtBodyScope(ASTScopeImpl *lookupParent, BraceStmt *body) + : lookupParent(lookupParent), body(body) {} SourceRange getSourceRangeOfThisASTNode(bool omitAssertions = false) const override; std::string getClassName() const override; +private: + void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &); + protected: ASTScopeImpl *expandSpecifically(ScopeCreator &) override; NullablePtr getLookupParent() const override { diff --git a/lib/AST/ASTScope.cpp b/lib/AST/ASTScope.cpp index 82e109614025b..ec6868c2c4412 100644 --- a/lib/AST/ASTScope.cpp +++ b/lib/AST/ASTScope.cpp @@ -137,8 +137,8 @@ DEFINE_GET_CLASS_NAME(DefaultArgumentInitializerScope) DEFINE_GET_CLASS_NAME(AttachedPropertyWrapperScope) DEFINE_GET_CLASS_NAME(PatternEntryDeclScope) DEFINE_GET_CLASS_NAME(PatternEntryInitializerScope) -DEFINE_GET_CLASS_NAME(ConditionalClauseScope) DEFINE_GET_CLASS_NAME(ConditionalClausePatternUseScope) +DEFINE_GET_CLASS_NAME(ConditionalClauseInitializerScope) DEFINE_GET_CLASS_NAME(CaptureListScope) DEFINE_GET_CLASS_NAME(ClosureParametersScope) DEFINE_GET_CLASS_NAME(TopLevelCodeScope) @@ -149,7 +149,7 @@ DEFINE_GET_CLASS_NAME(EnumElementScope) DEFINE_GET_CLASS_NAME(IfStmtScope) DEFINE_GET_CLASS_NAME(WhileStmtScope) DEFINE_GET_CLASS_NAME(GuardStmtScope) -DEFINE_GET_CLASS_NAME(LookupParentDiversionScope) +DEFINE_GET_CLASS_NAME(GuardStmtBodyScope) DEFINE_GET_CLASS_NAME(RepeatWhileScope) DEFINE_GET_CLASS_NAME(DoStmtScope) DEFINE_GET_CLASS_NAME(DoCatchStmtScope) @@ -199,15 +199,6 @@ void ASTScopeImpl::postOrderDo(function_ref fn) { fn(this); } -ArrayRef ConditionalClauseScope::getCond() const { - return stmt->getCond(); -} - -const StmtConditionElement & -ConditionalClauseScope::getStmtConditionElement() const { - return getCond()[index]; -} - unsigned ASTScopeImpl::countDescendants() const { unsigned count = 0; const_cast(this)->preOrderDo( diff --git a/lib/AST/ASTScopeCreation.cpp b/lib/AST/ASTScopeCreation.cpp index b7324c68013c7..a1673a0856016 100644 --- a/lib/AST/ASTScopeCreation.cpp +++ b/lib/AST/ASTScopeCreation.cpp @@ -687,6 +687,9 @@ class NodeAdder NullablePtr visitBraceStmt(BraceStmt *bs, ASTScopeImpl *p, ScopeCreator &scopeCreator) { + if (bs->empty()) + return p; + SmallVector localFuncsAndTypes; SmallVector localVars; @@ -941,17 +944,18 @@ ASTScopeImpl *ASTScopeImpl::expandAndBeCurrent(ScopeCreator &scopeCreator) { ASTScopeImpl *Scope::expandSpecifically(ScopeCreator &) { return this; } CREATES_NEW_INSERTION_POINT(ASTSourceFileScope) -CREATES_NEW_INSERTION_POINT(ConditionalClauseScope) CREATES_NEW_INSERTION_POINT(GuardStmtScope) CREATES_NEW_INSERTION_POINT(PatternEntryDeclScope) CREATES_NEW_INSERTION_POINT(GenericTypeOrExtensionScope) CREATES_NEW_INSERTION_POINT(BraceStmtScope) CREATES_NEW_INSERTION_POINT(TopLevelCodeScope) +CREATES_NEW_INSERTION_POINT(ConditionalClausePatternUseScope) NO_NEW_INSERTION_POINT(FunctionBodyScope) NO_NEW_INSERTION_POINT(AbstractFunctionDeclScope) NO_NEW_INSERTION_POINT(AttachedPropertyWrapperScope) NO_NEW_INSERTION_POINT(EnumElementScope) +NO_NEW_INSERTION_POINT(GuardStmtBodyScope) NO_NEW_INSERTION_POINT(ParameterListScope) NO_NEW_INSERTION_POINT(PatternEntryInitializerScope) @@ -959,6 +963,7 @@ NO_NEW_INSERTION_POINT(CaptureListScope) NO_NEW_INSERTION_POINT(CaseStmtScope) NO_NEW_INSERTION_POINT(CaseLabelItemScope) NO_NEW_INSERTION_POINT(CaseStmtBodyScope) +NO_NEW_INSERTION_POINT(ConditionalClauseInitializerScope) NO_NEW_INSERTION_POINT(ClosureParametersScope) NO_NEW_INSERTION_POINT(DefaultArgumentInitializerScope) NO_NEW_INSERTION_POINT(DoStmtScope) @@ -974,8 +979,6 @@ NO_NEW_INSERTION_POINT(WhileStmtScope) NO_EXPANSION(GenericParamScope) NO_EXPANSION(SpecializeAttributeScope) NO_EXPANSION(DifferentiableAttributeScope) -NO_EXPANSION(ConditionalClausePatternUseScope) -NO_EXPANSION(LookupParentDiversionScope) #undef CREATES_NEW_INSERTION_POINT #undef NO_NEW_INSERTION_POINT @@ -1057,42 +1060,49 @@ PatternEntryInitializerScope::expandAScopeThatDoesNotCreateANewInsertionPoint( this); } + AnnotatedInsertionPoint -ConditionalClauseScope::expandAScopeThatCreatesANewInsertionPoint( +ConditionalClausePatternUseScope::expandAScopeThatCreatesANewInsertionPoint( ScopeCreator &scopeCreator) { - const StmtConditionElement &sec = getStmtConditionElement(); - switch (sec.getKind()) { - case StmtConditionElement::CK_Availability: - return {this, "No introduced variables"}; - case StmtConditionElement::CK_Boolean: - scopeCreator.addToScopeTree(sec.getBoolean(), this); - return {this, "No introduced variables"}; - case StmtConditionElement::CK_PatternBinding: - scopeCreator.addToScopeTree(sec.getInitializer(), this); - auto *const ccPatternUseScope = - scopeCreator.constructExpandAndInsertUncheckable< - ConditionalClausePatternUseScope>(this, sec.getPattern(), endLoc); - return {ccPatternUseScope, - "Succeeding code must be in scope of conditional variables"}; - } - ASTScope_unreachable("Unhandled StmtConditionKind in switch"); + auto *initializer = sec.getInitializer(); + if (!isa(initializer)) { + scopeCreator + .constructExpandAndInsertUncheckable( + this, initializer); + } + + return {this, + "Succeeding code must be in scope of conditional clause pattern bindings"}; +} + +void +ConditionalClauseInitializerScope::expandAScopeThatDoesNotCreateANewInsertionPoint( + ScopeCreator &scopeCreator) { + scopeCreator.addToScopeTree(ASTNode(initializer), this); +} + +void +GuardStmtBodyScope::expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator & + scopeCreator) { + scopeCreator.addToScopeTree(ASTNode(body), this); } AnnotatedInsertionPoint GuardStmtScope::expandAScopeThatCreatesANewInsertionPoint(ScopeCreator & scopeCreator) { - ASTScopeImpl *conditionLookupParent = - createNestedConditionalClauseScopes(scopeCreator, stmt->getBody()); + createNestedConditionalClauseScopes(scopeCreator, endLoc); + // Add a child for the 'guard' body, which always exits. - // Parent is whole guard stmt scope, NOT the cond scopes - scopeCreator.addToScopeTree(stmt->getBody(), this); + // The lookup parent is whole guard stmt scope, NOT the cond scopes + auto *body = stmt->getBody(); + if (!body->empty()) { + scopeCreator + .constructExpandAndInsertUncheckable( + conditionLookupParent, this, stmt->getBody()); + } - auto *const lookupParentDiversionScope = - scopeCreator - .constructExpandAndInsertUncheckable( - this, conditionLookupParent, stmt->getEndLoc(), endLoc); - return {lookupParentDiversionScope, + return {conditionLookupParent, "Succeeding code must be in scope of guard variables"}; } @@ -1188,20 +1198,34 @@ void FunctionBodyScope::expandAScopeThatDoesNotCreateANewInsertionPoint( void IfStmtScope::expandAScopeThatDoesNotCreateANewInsertionPoint( ScopeCreator &scopeCreator) { + auto *thenStmt = stmt->getThenStmt(); + auto *elseStmt = stmt->getElseStmt(); + + SourceLoc endLoc = thenStmt->getEndLoc(); ASTScopeImpl *insertionPoint = - createNestedConditionalClauseScopes(scopeCreator, stmt->getThenStmt()); + createNestedConditionalClauseScopes(scopeCreator, endLoc); // The 'then' branch - scopeCreator.addToScopeTree(stmt->getThenStmt(), insertionPoint); + scopeCreator.addToScopeTree(thenStmt, insertionPoint); + + // Result builders can add an 'else' block consisting entirely of + // implicit expressions. In this case, the end location of the + // 'then' block is equal to the start location of the 'else' + // block, and the 'else' block source range is empty. + if (elseStmt && + thenStmt->getEndLoc() == elseStmt->getStartLoc() && + elseStmt->getStartLoc() == elseStmt->getEndLoc()) + return; // Add the 'else' branch, if needed. - scopeCreator.addToScopeTree(stmt->getElseStmt(), this); + scopeCreator.addToScopeTree(elseStmt, this); } void WhileStmtScope::expandAScopeThatDoesNotCreateANewInsertionPoint( ScopeCreator &scopeCreator) { + SourceLoc endLoc = stmt->getBody()->getEndLoc(); ASTScopeImpl *insertionPoint = - createNestedConditionalClauseScopes(scopeCreator, stmt->getBody()); + createNestedConditionalClauseScopes(scopeCreator, endLoc); scopeCreator.addToScopeTree(stmt->getBody(), insertionPoint); } @@ -1266,8 +1290,10 @@ void CaseStmtScope::expandAScopeThatDoesNotCreateANewInsertionPoint( } } - scopeCreator.constructExpandAndInsertUncheckable( - this, stmt); + if (!stmt->getBody()->empty()) { + scopeCreator.constructExpandAndInsertUncheckable( + this, stmt); + } } void CaseLabelItemScope::expandAScopeThatDoesNotCreateANewInsertionPoint( @@ -1409,14 +1435,23 @@ TypeAliasScope::createTrailingWhereClauseScope(ASTScopeImpl *parent, #pragma mark misc ASTScopeImpl *LabeledConditionalStmtScope::createNestedConditionalClauseScopes( - ScopeCreator &scopeCreator, const Stmt *const afterConds) { + ScopeCreator &scopeCreator, SourceLoc endLoc) { auto *stmt = getLabeledConditionalStmt(); ASTScopeImpl *insertionPoint = this; - for (unsigned i = 0; i < stmt->getCond().size(); ++i) { - insertionPoint = - scopeCreator - .constructExpandAndInsertUncheckable( - insertionPoint, stmt, i, afterConds->getStartLoc()); + for (auto &sec : stmt->getCond()) { + switch (sec.getKind()) { + case StmtConditionElement::CK_Availability: + break; + case StmtConditionElement::CK_Boolean: + scopeCreator.addToScopeTree(sec.getBoolean(), insertionPoint); + break; + case StmtConditionElement::CK_PatternBinding: + insertionPoint = + scopeCreator.constructExpandAndInsertUncheckable< + ConditionalClausePatternUseScope>( + insertionPoint, sec, endLoc); + break; + } } return insertionPoint; } diff --git a/lib/AST/ASTScopeLookup.cpp b/lib/AST/ASTScopeLookup.cpp index c49099bce99d2..396ddf8f658a3 100644 --- a/lib/AST/ASTScopeLookup.cpp +++ b/lib/AST/ASTScopeLookup.cpp @@ -272,7 +272,8 @@ bool GenericTypeOrExtensionScope::areMembersVisibleFromWhereClause() const { NullablePtr PatternEntryInitializerScope::getLookupParent() const { auto parent = getParent().get(); - assert(parent->getClassName() == "PatternEntryDeclScope"); + ASTScopeAssert(parent->getClassName() == "PatternEntryDeclScope", + "PatternEntryInitializerScope in unexpected place"); // Lookups from inside a pattern binding initializer skip the parent // scope that introduces bindings bound by the pattern, since we @@ -285,6 +286,23 @@ PatternEntryInitializerScope::getLookupParent() const { return parent->getLookupParent(); } +NullablePtr +ConditionalClauseInitializerScope::getLookupParent() const { + auto parent = getParent().get(); + ASTScopeAssert(parent->getClassName() == "ConditionalClausePatternUseScope", + "ConditionalClauseInitializerScope in unexpected place"); + + // Lookups from inside a conditional clause initializer skip the parent + // scope that introduces bindings bound by the pattern, since we + // want this to work: + // + // func f(x: Int?) { + // guard let x = x else { return } + // print(x) + // } + return parent->getLookupParent(); +} + #pragma mark looking in locals or members - locals bool GenericParamScope::lookupLocalsOrMembers(DeclConsumer consumer) const { @@ -411,7 +429,7 @@ bool ClosureParametersScope::lookupLocalsOrMembers( bool ConditionalClausePatternUseScope::lookupLocalsOrMembers( DeclConsumer consumer) const { - return lookupLocalBindingsInPattern(pattern, consumer); + return lookupLocalBindingsInPattern(sec.getPattern(), consumer); } bool ASTScopeImpl::lookupLocalBindingsInPattern(const Pattern *p, @@ -488,11 +506,7 @@ bool ASTScopeImpl::isLabeledStmtLookupTerminator() const { return true; } -bool LookupParentDiversionScope::isLabeledStmtLookupTerminator() const { - return false; -} - -bool ConditionalClauseScope::isLabeledStmtLookupTerminator() const { +bool GuardStmtBodyScope::isLabeledStmtLookupTerminator() const { return false; } diff --git a/lib/AST/ASTScopePrinting.cpp b/lib/AST/ASTScopePrinting.cpp index 2dafad62d3946..d300d24ded763 100644 --- a/lib/AST/ASTScopePrinting.cpp +++ b/lib/AST/ASTScopePrinting.cpp @@ -176,18 +176,13 @@ void AbstractPatternEntryScope::printSpecifics(llvm::raw_ostream &out) const { }); } -void ConditionalClauseScope::printSpecifics(llvm::raw_ostream &out) const { - ASTScopeImpl::printSpecifics(out); - out << "index " << index; -} - void SubscriptDeclScope::printSpecifics(llvm::raw_ostream &out) const { decl->dumpRef(out); } void ConditionalClausePatternUseScope::printSpecifics( llvm::raw_ostream &out) const { - pattern->print(out); + sec.getPattern()->print(out); } bool GenericTypeOrExtensionScope::doesDeclHaveABody() const { return false; } diff --git a/lib/AST/ASTScopeSourceRange.cpp b/lib/AST/ASTScopeSourceRange.cpp index c377febaef6e7..1fca63ca5bdd8 100644 --- a/lib/AST/ASTScopeSourceRange.cpp +++ b/lib/AST/ASTScopeSourceRange.cpp @@ -267,20 +267,14 @@ SourceRange PatternEntryInitializerScope::getSourceRangeOfThisASTNode( SourceRange GenericParamScope::getSourceRangeOfThisASTNode( const bool omitAssertions) const { - auto nOrE = holder; - // A protocol's generic parameter list is not written in source, and - // is visible from the start of the body. - if (auto *protoDecl = dyn_cast(nOrE)) - return SourceRange(protoDecl->getBraces().Start, protoDecl->getEndLoc()); - const auto startLoc = paramList->getSourceRange().Start; - const auto validStartLoc = - startLoc.isValid() ? startLoc : holder->getStartLoc(); - // Since ExtensionScope (whole portion) range doesn't start till after the - // extended nominal, the range here must be pushed back, too. + // We want to ensure the extended type is not part of the generic + // parameter scope. if (auto const *const ext = dyn_cast(holder)) { return SourceRange(getLocAfterExtendedNominal(ext), ext->getEndLoc()); } - return SourceRange(validStartLoc, holder->getEndLoc()); + + // For all other declarations, generic parameters are visible everywhere. + return holder->getSourceRange(); } SourceRange ASTSourceFileScope::getSourceRangeOfThisASTNode( @@ -412,21 +406,14 @@ BraceStmtScope::getSourceRangeOfThisASTNode(const bool omitAssertions) const { return stmt->getSourceRange(); } -SourceRange ConditionalClauseScope::getSourceRangeOfThisASTNode( +SourceRange ConditionalClauseInitializerScope::getSourceRangeOfThisASTNode( const bool omitAssertions) const { - // From the start of this particular condition to the start of the - // then/body part. - const auto startLoc = getStmtConditionElement().getStartLoc(); - return startLoc.isValid() - ? SourceRange(startLoc, endLoc) - : SourceRange(endLoc); + return initializer->getSourceRange(); } SourceRange ConditionalClausePatternUseScope::getSourceRangeOfThisASTNode( const bool omitAssertions) const { - // For a guard continuation, the scope extends from the end of the 'else' - // to the end of the continuation. - return SourceRange(startLoc); + return SourceRange(sec.getInitializer()->getStartLoc(), endLoc); } SourceRange @@ -456,9 +443,9 @@ SourceRange GuardStmtScope::getSourceRangeOfThisASTNode( return SourceRange(getStmt()->getStartLoc(), endLoc); } -SourceRange LookupParentDiversionScope::getSourceRangeOfThisASTNode( +SourceRange GuardStmtBodyScope::getSourceRangeOfThisASTNode( const bool omitAssertions) const { - return SourceRange(startLoc, endLoc); + return body->getSourceRange(); } #pragma mark source range caching diff --git a/test/NameLookup/scope_map-astscopelookup.swift b/test/NameLookup/scope_map-astscopelookup.swift index 7ae9c913028ea..dd72191c6fad2 100644 --- a/test/NameLookup/scope_map-astscopelookup.swift +++ b/test/NameLookup/scope_map-astscopelookup.swift @@ -308,7 +308,7 @@ func HasLabeledDo() { // CHECK-EXPANDED-NEXT: |-BraceStmtScope {{.*}}, [69:53 - 72:3] // CHECK-EXPANDED-NEXT: `-PatternEntryDeclScope {{.*}}, [70:9 - 70:13] entry 0 'c' // CHECK-EXPANDED-NEXT: `-PatternEntryInitializerScope {{.*}}, [70:13 - 70:13] entry 0 'c' -// CHECK-EXPANDED-NEXT: `-LookupParentDiversionScope, [72:3 - 100:38] +// CHECK-EXPANDED-NEXT: `-GuardStmtBodyScope, [72:3 - 100:38] // CHECK-EXPANDED-NEXT: |-WhileStmtScope {{.*}}, [74:3 - 76:3] // CHECK-EXPANDED-NEXT: `-ConditionalClauseScope, [74:9 - 76:3] index 0 // CHECK-EXPANDED-NEXT: `-ConditionalClausePatternUseScope, [74:21 - 76:3] let b3 diff --git a/test/NameLookup/scope_map_top_level.swift b/test/NameLookup/scope_map_top_level.swift index 014c920862e53..f4c22426d66e6 100644 --- a/test/NameLookup/scope_map_top_level.swift +++ b/test/NameLookup/scope_map_top_level.swift @@ -26,7 +26,7 @@ var i: Int = b.my_identity() // CHECK-EXPANDED: ***Complete scope map*** -// CHECK-EXPANDED-NEXT: ASTSourceFileScope {{.*}}, (uncached) [1:1 - 6{{.*}}:1] 'SOURCE_DIR{{[/\\]}}test{{[/\\]}}NameLookup{{[/\\]}}scope_map_top_level.swift' +// CHECK-EXPANDED-NEXT: ASTSourceFileScope {{.*}}, (uncached) [1:1 - 61:1] 'SOURCE_DIR{{[/\\]}}test{{[/\\]}}NameLookup{{[/\\]}}scope_map_top_level.swift' // CHECK-EXPANDED-NEXT: |-NominalTypeDeclScope {{.*}}, [4:1 - 4:13] // CHECK-EXPANDED-NEXT: `-NominalTypeBodyScope {{.*}}, [4:11 - 4:13] // CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [6:1 - 21:28] @@ -36,14 +36,11 @@ var i: Int = b.my_identity() // CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [8:1 - 21:28] // CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [8:1 - 21:28] // CHECK-EXPANDED-NEXT: `-GuardStmtScope {{.*}}, [8:1 - 21:28] -// CHECK-EXPANDED-NEXT: |-ConditionalClauseScope, [8:7 - 8:22] index 0 -// CHECK-EXPANDED-NEXT: `-ConditionalClausePatternUseScope, [8:22 - 8:22] let b{{\??}} -// CHECK-EXPANDED-NEXT: |-BraceStmtScope {{.*}}, [8:22 - 9:1] -// CHECK-EXPANDED-NEXT: `-LookupParentDiversionScope, [9:1 - 21:28] +// CHECK-EXPANDED-NEXT: `-ConditionalClausePatternUseScope, [8:15 - 21:28] let b{{\??}} +// CHECK-EXPANDED-NEXT: |-ConditionalClauseInitializerScope, [8:15 - 8:15] // CHECK-EXPANDED-NEXT: |-AbstractFunctionDeclScope {{.*}}, [11:1 - 11:19] 'foo(x:)' // CHECK-EXPANDED-NEXT: |-ParameterListScope {{.*}}, [11:9 - 11:16] // CHECK-EXPANDED-NEXT: `-FunctionBodyScope {{.*}}, [11:18 - 11:19] -// CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [11:18 - 11:19] // CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [13:1 - 21:28] // CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [13:1 - 21:28] // CHECK-EXPANDED-NEXT: |-PatternEntryDeclScope {{.*}}, [13:5 - 13:9] entry 0 'c' @@ -60,5 +57,4 @@ var i: Int = b.my_identity() // CHECK-EXPANDED-NEXT: `-PatternEntryInitializerScope {{.*}}, [21:14 - 21:28] entry 0 'i' -// REQUIRES: asserts -// absence of assertions can change the "uncached" bit and cause failures + From dac68ca0472de3debebd19a9c6473acfeb395419 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 2 Oct 2020 02:12:16 -0400 Subject: [PATCH 4/5] ASTScope: Fix TopLevelCodeScope source range Top-level code can contain guard statements which introduce bindings until the end of the parent scope, so plumb that through. --- include/swift/AST/ASTScope.h | 16 +++++++++++++--- lib/AST/ASTScopeCreation.cpp | 23 +++++++++++++++++------ lib/AST/ASTScopeSourceRange.cpp | 6 +++--- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/include/swift/AST/ASTScope.h b/include/swift/AST/ASTScope.h index 222b66a2167a8..1d4af06aa2ad0 100644 --- a/include/swift/AST/ASTScope.h +++ b/include/swift/AST/ASTScope.h @@ -1189,8 +1189,10 @@ class ClosureParametersScope final : public ASTScopeImpl { class TopLevelCodeScope final : public ASTScopeImpl { public: TopLevelCodeDecl *const decl; + SourceLoc endLoc; - TopLevelCodeScope(TopLevelCodeDecl *e) : decl(e) {} + TopLevelCodeScope(TopLevelCodeDecl *e, SourceLoc endLoc) + : decl(e), endLoc(endLoc) {} virtual ~TopLevelCodeScope() {} protected: @@ -1643,13 +1645,21 @@ class BraceStmtScope final : public AbstractStmtScope { /// definition. SmallVector localVars; + /// The end location for bindings introduced in this scope. This can + /// extend past the actual end of the BraceStmt in top-level code, + /// where every TopLevelCodeDecl introduces a new scope through the + /// end of the buffer. + SourceLoc endLoc; + public: BraceStmtScope(BraceStmt *e, SmallVector localFuncsAndTypes, - SmallVector localVars) + SmallVector localVars, + SourceLoc endLoc) : stmt(e), localFuncsAndTypes(localFuncsAndTypes), - localVars(localVars) {} + localVars(localVars), + endLoc(endLoc) {} virtual ~BraceStmtScope() {} protected: diff --git a/lib/AST/ASTScopeCreation.cpp b/lib/AST/ASTScopeCreation.cpp index a1673a0856016..1bb9fff0639a7 100644 --- a/lib/AST/ASTScopeCreation.cpp +++ b/lib/AST/ASTScopeCreation.cpp @@ -662,8 +662,9 @@ class NodeAdder NullablePtr visitTopLevelCodeDecl(TopLevelCodeDecl *d, ASTScopeImpl *p, ScopeCreator &scopeCreator) { - return scopeCreator.ifUniqueConstructExpandAndInsert(p, - d); + ASTScopeAssert(endLoc.hasValue(), "TopLevelCodeDecl in wrong place?"); + return scopeCreator.ifUniqueConstructExpandAndInsert( + p, d, *endLoc); } #pragma mark special-case creation @@ -708,9 +709,14 @@ class NodeAdder } } + SourceLoc endLocForBraceStmt = bs->getEndLoc(); + if (endLoc.hasValue()) + endLocForBraceStmt = *endLoc; + auto maybeBraceScope = scopeCreator.ifUniqueConstructExpandAndInsert( - p, bs, std::move(localFuncsAndTypes), std::move(localVars)); + p, bs, std::move(localFuncsAndTypes), std::move(localVars), + endLocForBraceStmt); if (auto *s = scopeCreator.getASTContext().Stats) ++s->getFrontendCounters().NumBraceStmtASTScopes; @@ -989,12 +995,17 @@ ASTSourceFileScope::expandAScopeThatCreatesANewInsertionPoint( ASTScopeAssert(SF, "Must already have a SourceFile."); ArrayRef decls = SF->getTopLevelDecls(); // Assume that decls are only added at the end, in source order + Optional endLoc = None; + if (!decls.empty()) + endLoc = decls.back()->getEndLoc(); + std::vector newNodes(decls.begin(), decls.end()); insertionPoint = scopeCreator.addSiblingsToScopeTree(insertionPoint, scopeCreator.sortBySourceRange( scopeCreator.cull(newNodes)), - None); + endLoc); + // Too slow to perform all the time: // ASTScopeAssert(scopeCreator->containsAllDeclContextsFromAST(), // "ASTScope tree missed some DeclContexts or made some up"); @@ -1123,7 +1134,7 @@ BraceStmtScope::expandAScopeThatCreatesANewInsertionPoint( scopeCreator.sortBySourceRange( scopeCreator.cull( stmt->getElements())), - stmt->getEndLoc()); + endLoc); if (auto *s = scopeCreator.getASTContext().Stats) ++s->getFrontendCounters().NumBraceStmtASTScopeExpansions; return { @@ -1137,7 +1148,7 @@ TopLevelCodeScope::expandAScopeThatCreatesANewInsertionPoint(ScopeCreator & if (auto *body = scopeCreator - .addToScopeTreeAndReturnInsertionPoint(decl->getBody(), this, None) + .addToScopeTreeAndReturnInsertionPoint(decl->getBody(), this, endLoc) .getPtrOrNull()) return {body, "So next top level code scope and put its decls in its body " "under a guard statement scope (etc) from the last top level " diff --git a/lib/AST/ASTScopeSourceRange.cpp b/lib/AST/ASTScopeSourceRange.cpp index 1fca63ca5bdd8..258631729f3a5 100644 --- a/lib/AST/ASTScopeSourceRange.cpp +++ b/lib/AST/ASTScopeSourceRange.cpp @@ -221,7 +221,7 @@ SourceRange FunctionBodyScope::getSourceRangeOfThisASTNode( SourceRange TopLevelCodeScope::getSourceRangeOfThisASTNode( const bool omitAssertions) const { - return decl->getSourceRange(); + return SourceRange(decl->getStartLoc(), endLoc); } SourceRange SubscriptDeclScope::getSourceRangeOfThisASTNode( @@ -401,9 +401,9 @@ BraceStmtScope::getSourceRangeOfThisASTNode(const bool omitAssertions) const { // 'in' keyword, when present. if (auto closure = parentClosureIfAny()) { if (closure.get()->getInLoc().isValid()) - return SourceRange(closure.get()->getInLoc(), stmt->getEndLoc()); + return SourceRange(closure.get()->getInLoc(), endLoc); } - return stmt->getSourceRange(); + return SourceRange(stmt->getStartLoc(), endLoc); } SourceRange ConditionalClauseInitializerScope::getSourceRangeOfThisASTNode( From b8cccb1ef83397a3e1b7267dfe27738cee90e19d Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 6 Oct 2020 00:55:12 -0400 Subject: [PATCH 5/5] ASTScope: Fix SourceFileScope source range A SourceFile might contain TopLevelCodeDecls with guard statements, which introduce names until the end of the file, so plumb that through. --- lib/AST/ASTScopeCreation.cpp | 6 ++---- test/NameLookup/scope_map_top_level.swift | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/lib/AST/ASTScopeCreation.cpp b/lib/AST/ASTScopeCreation.cpp index 1bb9fff0639a7..eda1e0c69cbb0 100644 --- a/lib/AST/ASTScopeCreation.cpp +++ b/lib/AST/ASTScopeCreation.cpp @@ -994,10 +994,8 @@ ASTSourceFileScope::expandAScopeThatCreatesANewInsertionPoint( ScopeCreator &scopeCreator) { ASTScopeAssert(SF, "Must already have a SourceFile."); ArrayRef decls = SF->getTopLevelDecls(); - // Assume that decls are only added at the end, in source order - Optional endLoc = None; - if (!decls.empty()) - endLoc = decls.back()->getEndLoc(); + + SourceLoc endLoc = getSourceRangeOfThisASTNode().End; std::vector newNodes(decls.begin(), decls.end()); insertionPoint = diff --git a/test/NameLookup/scope_map_top_level.swift b/test/NameLookup/scope_map_top_level.swift index f4c22426d66e6..07f94a67846bc 100644 --- a/test/NameLookup/scope_map_top_level.swift +++ b/test/NameLookup/scope_map_top_level.swift @@ -29,20 +29,20 @@ var i: Int = b.my_identity() // CHECK-EXPANDED-NEXT: ASTSourceFileScope {{.*}}, (uncached) [1:1 - 61:1] 'SOURCE_DIR{{[/\\]}}test{{[/\\]}}NameLookup{{[/\\]}}scope_map_top_level.swift' // CHECK-EXPANDED-NEXT: |-NominalTypeDeclScope {{.*}}, [4:1 - 4:13] // CHECK-EXPANDED-NEXT: `-NominalTypeBodyScope {{.*}}, [4:11 - 4:13] -// CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [6:1 - 21:28] -// CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [6:1 - 21:28] +// CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [6:1 - 61:1] +// CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [6:1 - 61:1] // CHECK-EXPANDED-NEXT: |-PatternEntryDeclScope {{.*}}, [6:5 - 6:15] entry 0 'a' // CHECK-EXPANDED-NEXT: `-PatternEntryInitializerScope {{.*}}, [6:15 - 6:15] entry 0 'a' -// CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [8:1 - 21:28] -// CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [8:1 - 21:28] -// CHECK-EXPANDED-NEXT: `-GuardStmtScope {{.*}}, [8:1 - 21:28] -// CHECK-EXPANDED-NEXT: `-ConditionalClausePatternUseScope, [8:15 - 21:28] let b{{\??}} +// CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [8:1 - 61:1] +// CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [8:1 - 61:1] +// CHECK-EXPANDED-NEXT: `-GuardStmtScope {{.*}}, [8:1 - 61:1] +// CHECK-EXPANDED-NEXT: `-ConditionalClausePatternUseScope, [8:15 - 61:1] let b{{\??}} // CHECK-EXPANDED-NEXT: |-ConditionalClauseInitializerScope, [8:15 - 8:15] // CHECK-EXPANDED-NEXT: |-AbstractFunctionDeclScope {{.*}}, [11:1 - 11:19] 'foo(x:)' // CHECK-EXPANDED-NEXT: |-ParameterListScope {{.*}}, [11:9 - 11:16] // CHECK-EXPANDED-NEXT: `-FunctionBodyScope {{.*}}, [11:18 - 11:19] -// CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [13:1 - 21:28] -// CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [13:1 - 21:28] +// CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [13:1 - 61:1] +// CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [13:1 - 61:1] // CHECK-EXPANDED-NEXT: |-PatternEntryDeclScope {{.*}}, [13:5 - 13:9] entry 0 'c' // CHECK-EXPANDED-NEXT: `-PatternEntryInitializerScope {{.*}}, [13:9 - 13:9] entry 0 'c' // CHECK-EXPANDED-NEXT: |-TypeAliasDeclScope {{.*}}, [15:1 - 15:15] @@ -51,8 +51,8 @@ var i: Int = b.my_identity() // CHECK-EXPANDED-NEXT: `-AbstractFunctionDeclScope {{.*}}, [18:3 - 18:43] 'my_identity()' // CHECK-EXPANDED-NEXT: `-FunctionBodyScope {{.*}}, [18:29 - 18:43] // CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [18:29 - 18:43] -// CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [21:1 - 21:28] -// CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [21:1 - 21:28] +// CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [21:1 - 61:1] +// CHECK-EXPANDED-NEXT: `-BraceStmtScope {{.*}}, [21:1 - 61:1] // CHECK-EXPANDED-NEXT: `-PatternEntryDeclScope {{.*}}, [21:5 - 21:28] entry 0 'i' // CHECK-EXPANDED-NEXT: `-PatternEntryInitializerScope {{.*}}, [21:14 - 21:28] entry 0 'i'