diff --git a/include/swift/AST/ASTScope.h b/include/swift/AST/ASTScope.h index a1145d3f96e45..4495015674112 100644 --- a/include/swift/AST/ASTScope.h +++ b/include/swift/AST/ASTScope.h @@ -397,7 +397,6 @@ class ASTSourceFileScope final : public ASTScopeImpl { public: SourceFile *const SF; ScopeCreator *const scopeCreator; - ASTScopeImpl *insertionPoint; ASTSourceFileScope(SourceFile *SF, ScopeCreator *scopeCreator); @@ -798,7 +797,6 @@ class ParameterListScope final : public ASTScopeImpl { private: void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &); - SourceLoc fixupEndForBadInput(SourceRange) const; public: std::string getClassName() const override; diff --git a/lib/AST/ASTScopeCreation.cpp b/lib/AST/ASTScopeCreation.cpp index 1dfaf3c5eb6ab..c7fd9f941cea2 100644 --- a/lib/AST/ASTScopeCreation.cpp +++ b/lib/AST/ASTScopeCreation.cpp @@ -39,96 +39,6 @@ using namespace swift; using namespace ast_scope; -#pragma mark source range utilities -static bool rangeableIsIgnored(const Decl *d) { return d->isImplicit(); } -static bool rangeableIsIgnored(const Expr *d) { - return false; // implicit expr may contain closures -} -static bool rangeableIsIgnored(const Stmt *d) { - return false; // ?? -} -static bool rangeableIsIgnored(const ASTNode n) { - return (n.is() && rangeableIsIgnored(n.get())) || - (n.is() && rangeableIsIgnored(n.get())) || - (n.is() && rangeableIsIgnored(n.get())); -} - -template -static SourceRange getRangeableSourceRange(const Rangeable *const p) { - return p->getSourceRange(); -} -static SourceRange getRangeableSourceRange(const ASTNode n) { - return n.getSourceRange(); -} - -template -static bool isLocalizable(const Rangeable astElement) { - return !rangeableIsIgnored(astElement) && - getRangeableSourceRange(astElement).isValid(); -} - -template -static void dumpRangeable(const Rangeable r, llvm::raw_ostream &f) { - r.dump(f); -} -template -static void dumpRangeable(const Rangeable *r, llvm::raw_ostream &f) { - r->dump(f); -} -template -static void dumpRangeable(Rangeable *r, llvm::raw_ostream &f) { - r->dump(f); -} - -static void dumpRangeable(const SpecializeAttr *r, - llvm::raw_ostream &f) LLVM_ATTRIBUTE_USED; -static void dumpRangeable(const SpecializeAttr *r, llvm::raw_ostream &f) { - llvm::errs() << "SpecializeAttr\n"; -} -static void dumpRangeable(SpecializeAttr *r, - llvm::raw_ostream &f) LLVM_ATTRIBUTE_USED; -static void dumpRangeable(SpecializeAttr *r, llvm::raw_ostream &f) { - llvm::errs() << "SpecializeAttr\n"; -} - -static void dumpRangeable(const DifferentiableAttr *a, - llvm::raw_ostream &f) LLVM_ATTRIBUTE_USED; -static void dumpRangeable(const DifferentiableAttr *a, llvm::raw_ostream &f) { - llvm::errs() << "DifferentiableAttr\n"; -} -static void dumpRangeable(DifferentiableAttr *a, - llvm::raw_ostream &f) LLVM_ATTRIBUTE_USED; -static void dumpRangeable(DifferentiableAttr *a, llvm::raw_ostream &f) { - llvm::errs() << "DifferentiableAttr\n"; -} - -/// For Debugging -template -bool doesRangeableRangeMatch(const T *x, const SourceManager &SM, - unsigned start, unsigned end, - StringRef file = "") { - auto const r = getRangeableSourceRange(x); - if (r.isInvalid()) - return false; - if (start && SM.getLineAndColumnInBuffer(r.Start).first != start) - return false; - if (end && SM.getLineAndColumnInBuffer(r.End).first != end) - return false; - if (file.empty()) - return true; - const auto buf = SM.findBufferContainingLoc(r.Start); - return SM.getIdentifierForBuffer(buf).endswith(file); -} - -#pragma mark end of rangeable - -static std::vector asNodeVector(DeclRange dr) { - std::vector nodes; - llvm::transform(dr, std::back_inserter(nodes), - [&](Decl *d) { return ASTNode(d); }); - return nodes; -} - namespace swift { namespace ast_scope { @@ -150,22 +60,6 @@ class ScopeCreator final { ScopeCreator(const ScopeCreator &) = delete; // ensure no copies ScopeCreator(const ScopeCreator &&) = delete; // ensure no moves - /// Given an array of ASTNodes or Decl pointers, add them - /// Return the resultant insertionPoint - /// - /// \param endLoc The end location for any "scopes until the end" that - /// we introduce here, such as PatternEntryDeclScope and GuardStmtScope - ASTScopeImpl * - addSiblingsToScopeTree(ASTScopeImpl *const insertionPoint, - ArrayRef nodesOrDeclsToAdd, - Optional endLoc) { - auto *ip = insertionPoint; - for (auto nd : nodesOrDeclsToAdd) { - ip = addToScopeTreeAndReturnInsertionPoint(nd, ip, endLoc); - } - return ip; - } - public: /// For each of searching, call this unless the insertion point is needed void addToScopeTree(ASTNode n, ASTScopeImpl *parent) { @@ -180,62 +74,6 @@ class ScopeCreator final { addToScopeTreeAndReturnInsertionPoint(ASTNode, ASTScopeImpl *parent, Optional endLoc); - bool isWorthTryingToCreateScopeFor(ASTNode n) const { - if (!n) - return false; - if (n.is()) - return true; - // Cannot ignore implicit statements because implict return can contain - // scopes in the expression, such as closures. - // But must ignore other implicit statements, e.g. brace statments - // if they can have no children and no stmt source range. - // Deal with it in visitBraceStmt - if (n.is()) - return true; - - auto *const d = n.get(); - // Implicit nodes may not have source information for name lookup. - if (!isLocalizable(d)) - return false; - - // Commented out for - // validation-test/compiler_crashers_fixed/27962-swift-rebindselfinconstructorexpr-getcalledconstructor.swift - // In that test the invalid PBD -> var decl which contains the desired - // closure scope - // if (const auto *PBD = dyn_cast(d)) - // if (!isLocalizable(PBD)) - // return false; - /// In - /// \code - /// @propertyWrapper - /// public struct Wrapper { - /// public var value: T - /// - /// public init(body: () -> T) { - /// self.value = body() - /// } - /// } - /// - /// let globalInt = 17 - /// - /// @Wrapper(body: { globalInt }) - /// public var y: Int - /// \endcode - /// I'm seeing a dumped AST include: - /// (pattern_binding_decl range=[test.swift:13:8 - line:12:29] - const auto &SM = d->getASTContext().SourceMgr; - - // Once we allow invalid PatternBindingDecls (see - // isWorthTryingToCreateScopeFor), then - // IDE/complete_property_delegate_attribute.swift fails because we try to - // expand a member whose source range is backwards. - (void)SM; - ASTScopeAssert(d->getStartLoc().isInvalid() || - !SM.isBeforeInBuffer(d->getEndLoc(), d->getStartLoc()), - "end-before-start will break tree search via location"); - return true; - } - template ASTScopeImpl *constructExpandAndInsert(ASTScopeImpl *parent, Args... args) { auto *child = new (ctx) Scope(args...); @@ -321,8 +159,8 @@ class ScopeCreator final { } void - addChildrenForAllLocalizableAccessorsInSourceOrder(AbstractStorageDecl *asd, - ASTScopeImpl *parent); + addChildrenForParsedAccessors(AbstractStorageDecl *asd, + ASTScopeImpl *parent); void addChildrenForKnownAttributes(ValueDecl *decl, ASTScopeImpl *parent); @@ -339,26 +177,6 @@ class ScopeCreator final { ASTScopeImpl *parent, Optional endLoc); - /// Remove VarDecls because we'll find them when we expand the - /// PatternBindingDecls. Remove EnunCases - /// because they overlap EnumElements and AST includes the elements in the - /// members. - std::vector cull(ArrayRef input) const { - // TODO: Investigate whether to move the real EndLoc tracking of - // SubscriptDecl up into AbstractStorageDecl. May have to cull more. - std::vector culled; - llvm::copy_if(input, std::back_inserter(culled), [&](ASTNode n) { - ASTScopeAssert( - !n.isDecl(DeclKind::Accessor), - "Should not find accessors in iterable types or brace statements"); - return isLocalizable(n) && - !n.isDecl(DeclKind::Var) && - !n.isDecl(DeclKind::EnumCase) && - !n.isDecl(DeclKind::IfConfig); - }); - return culled; - } - SWIFT_DEBUG_DUMP { print(llvm::errs()); } void print(raw_ostream &out) const { @@ -429,7 +247,7 @@ void ASTSourceFileScope::expandFunctionBody(AbstractFunctionDecl *AFD) { ASTSourceFileScope::ASTSourceFileScope(SourceFile *SF, ScopeCreator *scopeCreator) - : SF(SF), scopeCreator(scopeCreator), insertionPoint(this) {} + : SF(SF), scopeCreator(scopeCreator) {} #pragma mark NodeAdder @@ -466,6 +284,10 @@ class NodeAdder VISIT_AND_IGNORE(PoundDiagnosticDecl) VISIT_AND_IGNORE(MissingMemberDecl) + // Only members of the active clause are in scope, and those + // are visited separately. + VISIT_AND_IGNORE(IfConfigDecl) + // This declaration is handled from the PatternBindingDecl VISIT_AND_IGNORE(VarDecl) @@ -517,7 +339,7 @@ class NodeAdder #undef VISIT_AND_CREATE_WHOLE_PORTION // This declaration is handled from - // addChildrenForAllLocalizableAccessorsInSourceOrder + // addChildrenForParsedAccessors ASTScopeImpl *visitAccessorDecl(AccessorDecl *ad, ASTScopeImpl *p, ScopeCreator &scopeCreator) { return visitAbstractFunctionDecl(ad, p, scopeCreator); @@ -612,14 +434,6 @@ class NodeAdder return p; } - ASTScopeImpl *visitIfConfigDecl(IfConfigDecl *icd, - ASTScopeImpl *p, - ScopeCreator &scopeCreator) { - ASTScope_unreachable( - "Should be handled inside of " - "expandIfConfigClausesThenCullAndSortElementsOrMembers"); - } - ASTScopeImpl *visitReturnStmt(ReturnStmt *rs, ASTScopeImpl *p, ScopeCreator &scopeCreator) { if (rs->hasResult()) @@ -657,9 +471,13 @@ ASTScopeImpl * ScopeCreator::addToScopeTreeAndReturnInsertionPoint(ASTNode n, ASTScopeImpl *parent, Optional endLoc) { - if (!isWorthTryingToCreateScopeFor(n)) + if (!n) return parent; + if (auto *d = n.dyn_cast()) + if (d->isImplicit()) + return parent; + NodeAdder adder(endLoc); if (auto *p = n.dyn_cast()) return adder.visit(p, parent, *this); @@ -669,7 +487,7 @@ ScopeCreator::addToScopeTreeAndReturnInsertionPoint(ASTNode n, return adder.visit(p, parent, *this); } -void ScopeCreator::addChildrenForAllLocalizableAccessorsInSourceOrder( +void ScopeCreator::addChildrenForParsedAccessors( AbstractStorageDecl *asd, ASTScopeImpl *parent) { asd->visitParsedAccessors([&](AccessorDecl *ad) { assert(asd == ad->getStorage()); @@ -867,19 +685,15 @@ AnnotatedInsertionPoint ASTSourceFileScope::expandAScopeThatCreatesANewInsertionPoint( ScopeCreator &scopeCreator) { ASTScopeAssert(SF, "Must already have a SourceFile."); - ArrayRef decls = SF->getTopLevelDecls(); SourceLoc endLoc = getSourceRangeOfThisASTNode().End; - std::vector newNodes(decls.begin(), decls.end()); - insertionPoint = - scopeCreator.addSiblingsToScopeTree(insertionPoint, - scopeCreator.cull(newNodes), - endLoc); + ASTScopeImpl *insertionPoint = this; + for (auto *d : SF->getTopLevelDecls()) { + insertionPoint = scopeCreator.addToScopeTreeAndReturnInsertionPoint( + ASTNode(d), insertionPoint, endLoc); + } - // Too slow to perform all the time: - // ASTScopeAssert(scopeCreator->containsAllDeclContextsFromAST(), - // "ASTScope tree missed some DeclContexts or made some up"); return {insertionPoint, "Next time decls are added they go here."}; } @@ -909,8 +723,10 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint( // so compute it ourselves. // Even if this predicate fails, there may be an initContext but // we cannot make a scope for it, since no source range. - if (patternEntry.getOriginalInit() && - isLocalizable(patternEntry.getOriginalInit())) { + if (patternEntry.getOriginalInit()) { + ASTScopeAssert( + patternEntry.getOriginalInit()->getSourceRange().isValid(), + "pattern initializer has invalid source range"); ASTScopeAssert( !getSourceManager().isBeforeInBuffer( patternEntry.getOriginalInit()->getStartLoc(), decl->getStartLoc()), @@ -922,7 +738,7 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint( // Add accessors for the variables in this pattern. patternEntry.getPattern()->forEachVariable([&](VarDecl *var) { - scopeCreator.addChildrenForAllLocalizableAccessorsInSourceOrder(var, this); + scopeCreator.addChildrenForParsedAccessors(var, this); }); // In local context, the PatternEntryDeclScope becomes the insertion point, so @@ -998,15 +814,15 @@ GenericTypeOrExtensionScope::expandAScopeThatCreatesANewInsertionPoint( AnnotatedInsertionPoint BraceStmtScope::expandAScopeThatCreatesANewInsertionPoint( ScopeCreator &scopeCreator) { - // TODO: remove the sort after fixing parser to create brace statement - // elements in source order - auto *insertionPoint = - scopeCreator.addSiblingsToScopeTree(this, - scopeCreator.cull( - stmt->getElements()), - endLoc); + ASTScopeImpl *insertionPoint = this; + for (auto nd : stmt->getElements()) { + insertionPoint = scopeCreator.addToScopeTreeAndReturnInsertionPoint( + nd, insertionPoint, endLoc); + } + if (auto *s = scopeCreator.getASTContext().Stats) ++s->getFrontendCounters().NumBraceStmtASTScopeExpansions; + return { insertionPoint, "For top-level code decls, need the scope under, say a guard statment."}; @@ -1131,8 +947,10 @@ void SwitchStmtScope::expandAScopeThatDoesNotCreateANewInsertionPoint( scopeCreator.addToScopeTree(stmt->getSubjectExpr(), this); for (auto caseStmt : stmt->getCases()) { - if (isLocalizable(caseStmt)) - scopeCreator.constructExpandAndInsert(this, caseStmt); + ASTScopeAssert( + caseStmt->getSourceRange().isValid(), + "pattern initializer has invalid source range"); + scopeCreator.constructExpandAndInsert(this, caseStmt); } } @@ -1146,8 +964,10 @@ void ForEachStmtScope::expandAScopeThatDoesNotCreateANewInsertionPoint( // the body is implicit and it would overlap the source range of the expr // above. if (!stmt->getBody()->isImplicit()) { - if (isLocalizable(stmt->getBody())) - scopeCreator.constructExpandAndInsert(this, stmt); + ASTScopeAssert( + stmt->getBody()->getSourceRange().isValid(), + "pattern initializer has invalid source range"); + scopeCreator.constructExpandAndInsert(this, stmt); } } @@ -1187,7 +1007,7 @@ void SubscriptDeclScope::expandAScopeThatDoesNotCreateANewInsertionPoint( decl, decl->getGenericParams(), this); scopeCreator.constructExpandAndInsert( leaf, decl->getIndices(), decl->getAccessor(AccessorKind::Get)); - scopeCreator.addChildrenForAllLocalizableAccessorsInSourceOrder(decl, leaf); + scopeCreator.addChildrenForParsedAccessors(decl, leaf); } void CaptureListScope::expandAScopeThatDoesNotCreateANewInsertionPoint( @@ -1364,9 +1184,9 @@ void FunctionBodyScope::expandBody(ScopeCreator &scopeCreator) { void GenericTypeOrExtensionScope::expandBody(ScopeCreator &) {} void IterableTypeScope::expandBody(ScopeCreator &scopeCreator) { - auto nodes = asNodeVector(getIterableDeclContext().get()->getMembers()); - nodes = scopeCreator.cull(nodes); - scopeCreator.addSiblingsToScopeTree(this, nodes, None); + for (auto *d : getIterableDeclContext().get()->getMembers()) + scopeCreator.addToScopeTree(ASTNode(d), this); + if (auto *s = scopeCreator.getASTContext().Stats) ++s->getFrontendCounters().NumIterableTypeBodyASTScopeExpansions; } diff --git a/lib/AST/ASTScopeSourceRange.cpp b/lib/AST/ASTScopeSourceRange.cpp index ecd6892623987..7cbc5ce584403 100644 --- a/lib/AST/ASTScopeSourceRange.cpp +++ b/lib/AST/ASTScopeSourceRange.cpp @@ -128,19 +128,14 @@ SourceRange AbstractStmtScope::getSourceRangeOfThisASTNode( SourceRange DefaultArgumentInitializerScope::getSourceRangeOfThisASTNode( const bool omitAssertions) const { - if (auto *dv = decl->getStructuralDefaultExpr()) - return dv->getSourceRange(); - return SourceRange(); + return decl->getStructuralDefaultExpr()->getSourceRange(); } SourceRange PatternEntryDeclScope::getSourceRangeOfThisASTNode( const bool omitAssertions) const { SourceRange range = getPatternEntry().getSourceRange(); - if (endLoc.hasValue()) { - ASTScopeAssert(endLoc->isValid(), - "BraceStmt ends before pattern binding entry?"); + if (endLoc.hasValue()) range.End = *endLoc; - } return range; } @@ -244,20 +239,7 @@ SourceRange AbstractFunctionDeclScope::getSourceRangeOfThisASTNode( SourceRange ParameterListScope::getSourceRangeOfThisASTNode( const bool omitAssertions) const { - auto rangeForGoodInput = params->getSourceRange(); - auto r = SourceRange(rangeForGoodInput.Start, - fixupEndForBadInput(rangeForGoodInput)); - ASTScopeAssert(getSourceManager().rangeContains( - getParent().get()->getSourceRangeOfThisASTNode(true), r), - "Parameters not within function?!"); - return r; -} - -SourceLoc ParameterListScope::fixupEndForBadInput( - const SourceRange rangeForGoodInput) const { - const auto s = rangeForGoodInput.Start; - const auto e = rangeForGoodInput.End; - return getSourceManager().isBeforeInBuffer(s, e) ? e : s; + return params->getSourceRange(); } SourceRange ForEachPatternScope::getSourceRangeOfThisASTNode( diff --git a/lib/AST/DeclContext.cpp b/lib/AST/DeclContext.cpp index 2ef4cd00deb9c..8edd07131b887 100644 --- a/lib/AST/DeclContext.cpp +++ b/lib/AST/DeclContext.cpp @@ -774,8 +774,18 @@ void IterableDeclContext::addMemberPreservingSourceOrder(Decl *member) { if (existingMember->isImplicit()) continue; - if (isa(existingMember) || - isa(existingMember)) + // An EnumCaseDecl contains one or more EnumElementDecls, + // but the EnumElementDecls are also added as members of + // the parent enum. We ignore the EnumCaseDecl since its + // source range overlaps with that of the EnumElementDecls. + if (isa(existingMember)) + continue; + + // The elements of the active clause of an IfConfigDecl + // are added to the parent type. We ignore the IfConfigDecl + // since its source range overlaps with the source ranges + // of the active elements. + if (isa(existingMember)) continue; if (!SM.isBeforeInBuffer(existingMember->getEndLoc(), start)) @@ -819,14 +829,24 @@ void IterableDeclContext::addMemberSilently(Decl *member, Decl *hint, assert(!member->NextDecl && "Already added to a container"); #ifndef NDEBUG + // Assert that new declarations are always added in source order. auto checkSourceRange = [&](Decl *prev, Decl *next) { + // SKip these checks for imported and deserialized decls. if (!member->getDeclContext()->getParentSourceFile()) return; auto shouldSkip = [](Decl *d) { - if (isa(d) || isa(d) || isa(d)) + // PatternBindingDecl source ranges overlap with VarDecls, + // EnumCaseDecl source ranges overlap with EnumElementDecls, + // and IfConfigDecl source ranges overlap with the elements + // of the active clause. Skip them all here to avoid + // spurious assertions. + if (isa(d) || + isa(d) || + isa(d)) return true; + // Ignore source location information of implicit declarations. if (d->isImplicit()) return true; @@ -839,8 +859,10 @@ void IterableDeclContext::addMemberSilently(Decl *member, Decl *hint, SourceLoc prevEnd = prev->getEndLoc(); SourceLoc nextStart = next->getStartLoc(); - if (!prevEnd.isValid() || !nextStart.isValid()) - return; + assert(prevEnd.isValid() && + "Only implicit decls can have invalid source location"); + assert(nextStart.isValid() && + "Only implicit decls can have invalid source location"); if (getASTContext().SourceMgr.isBeforeInBuffer(prevEnd, nextStart)) return;