From c5e138824544531fdf55935061afea1e999cc4b1 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 7 Oct 2020 16:45:39 -0400 Subject: [PATCH 1/6] ASTScope: Remove cull() --- lib/AST/ASTScopeCreation.cpp | 101 ++++------------------------------- 1 file changed, 11 insertions(+), 90 deletions(-) diff --git a/lib/AST/ASTScopeCreation.cpp b/lib/AST/ASTScopeCreation.cpp index 1dfaf3c5eb6ab..f8fd455e97bb3 100644 --- a/lib/AST/ASTScopeCreation.cpp +++ b/lib/AST/ASTScopeCreation.cpp @@ -180,62 +180,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...); @@ -339,26 +283,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 { @@ -466,6 +390,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) @@ -612,14 +540,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 +577,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); @@ -874,8 +798,7 @@ ASTSourceFileScope::expandAScopeThatCreatesANewInsertionPoint( std::vector newNodes(decls.begin(), decls.end()); insertionPoint = scopeCreator.addSiblingsToScopeTree(insertionPoint, - scopeCreator.cull(newNodes), - endLoc); + newNodes, endLoc); // Too slow to perform all the time: // ASTScopeAssert(scopeCreator->containsAllDeclContextsFromAST(), @@ -1002,8 +925,7 @@ BraceStmtScope::expandAScopeThatCreatesANewInsertionPoint( // elements in source order auto *insertionPoint = scopeCreator.addSiblingsToScopeTree(this, - scopeCreator.cull( - stmt->getElements()), + stmt->getElements(), endLoc); if (auto *s = scopeCreator.getASTContext().Stats) ++s->getFrontendCounters().NumBraceStmtASTScopeExpansions; @@ -1365,7 +1287,6 @@ void GenericTypeOrExtensionScope::expandBody(ScopeCreator &) {} void IterableTypeScope::expandBody(ScopeCreator &scopeCreator) { auto nodes = asNodeVector(getIterableDeclContext().get()->getMembers()); - nodes = scopeCreator.cull(nodes); scopeCreator.addSiblingsToScopeTree(this, nodes, None); if (auto *s = scopeCreator.getASTContext().Stats) ++s->getFrontendCounters().NumIterableTypeBodyASTScopeExpansions; From 89ea51e90ce69e62cd169b453db2e2d4d63b6728 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 7 Oct 2020 17:11:26 -0400 Subject: [PATCH 2/6] ASTScope: Remove isLocalizable() --- lib/AST/ASTScopeCreation.cpp | 101 +++++------------------------------ 1 file changed, 12 insertions(+), 89 deletions(-) diff --git a/lib/AST/ASTScopeCreation.cpp b/lib/AST/ASTScopeCreation.cpp index f8fd455e97bb3..34cb726f9ba82 100644 --- a/lib/AST/ASTScopeCreation.cpp +++ b/lib/AST/ASTScopeCreation.cpp @@ -39,89 +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), @@ -832,8 +749,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()), @@ -1053,8 +972,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); } } @@ -1068,8 +989,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); } } From ea9f84f66ee9f010cbedb2eff17e956ffe5db450 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 7 Oct 2020 17:20:22 -0400 Subject: [PATCH 3/6] ASTScope: Remove addSiblingsToScopeTree() --- include/swift/AST/ASTScope.h | 1 - lib/AST/ASTScopeCreation.cpp | 56 +++++++++++------------------------- 2 files changed, 16 insertions(+), 41 deletions(-) diff --git a/include/swift/AST/ASTScope.h b/include/swift/AST/ASTScope.h index a1145d3f96e45..6bf8e6857440c 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); diff --git a/lib/AST/ASTScopeCreation.cpp b/lib/AST/ASTScopeCreation.cpp index 34cb726f9ba82..9a947dfd4b58c 100644 --- a/lib/AST/ASTScopeCreation.cpp +++ b/lib/AST/ASTScopeCreation.cpp @@ -39,13 +39,6 @@ using namespace swift; using namespace ast_scope; -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 { @@ -67,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) { @@ -270,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 @@ -708,18 +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, - 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."}; } @@ -840,14 +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, - 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."}; @@ -1209,8 +1184,9 @@ void FunctionBodyScope::expandBody(ScopeCreator &scopeCreator) { void GenericTypeOrExtensionScope::expandBody(ScopeCreator &) {} void IterableTypeScope::expandBody(ScopeCreator &scopeCreator) { - auto nodes = asNodeVector(getIterableDeclContext().get()->getMembers()); - 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; } From 885f6ebba3c39fe2b097631769b2ac8451e01e82 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 7 Oct 2020 17:20:34 -0400 Subject: [PATCH 4/6] ASTScope: Rename addChildrenForAllLocalizableAccessorsInSourceOrder() --- lib/AST/ASTScopeCreation.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/AST/ASTScopeCreation.cpp b/lib/AST/ASTScopeCreation.cpp index 9a947dfd4b58c..c7fd9f941cea2 100644 --- a/lib/AST/ASTScopeCreation.cpp +++ b/lib/AST/ASTScopeCreation.cpp @@ -159,8 +159,8 @@ class ScopeCreator final { } void - addChildrenForAllLocalizableAccessorsInSourceOrder(AbstractStorageDecl *asd, - ASTScopeImpl *parent); + addChildrenForParsedAccessors(AbstractStorageDecl *asd, + ASTScopeImpl *parent); void addChildrenForKnownAttributes(ValueDecl *decl, ASTScopeImpl *parent); @@ -339,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); @@ -487,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()); @@ -738,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 @@ -1007,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( From b720c04f4b8f03dc93a00943ca9c3048f6305d42 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 7 Oct 2020 17:29:53 -0400 Subject: [PATCH 5/6] ASTScope: Simplify a couple of getSourceRangeOfThisASTNode() methods --- include/swift/AST/ASTScope.h | 1 - lib/AST/ASTScopeSourceRange.cpp | 24 +++--------------------- 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/include/swift/AST/ASTScope.h b/include/swift/AST/ASTScope.h index 6bf8e6857440c..4495015674112 100644 --- a/include/swift/AST/ASTScope.h +++ b/include/swift/AST/ASTScope.h @@ -797,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/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( From 4b0d39c49d96c32f1ec0a1bdc8b54c9ef5c7365f Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 8 Oct 2020 16:15:21 -0400 Subject: [PATCH 6/6] AST: Clean up and write better comments for source range assertions in addMember() --- lib/AST/DeclContext.cpp | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) 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;