Merge pull request swiftlang#34207 from slavapestov/astscope-char-source-ranges

Migrate ASTScope to CharSourceRanges

Author: slavapestov

include/swift/AST/ASTScope.h

@@ -141,21 +141,14 @@ class ASTScopeImpl {
   ASTScopeImpl *parent = nullptr; // null at the root
 
   /// Child scopes, sorted by source range.
-  /// Must clear source range change whenever this changes
   Children storedChildren;
 
   bool wasExpanded = false;
 
   /// Can clear storedChildren, so must remember this
   bool haveAddedCleanup = false;
 
-  // Must be updated after last child is added and after last child's source
-  // position is known
-  mutable Optional<SourceRange> cachedSourceRange;
-
-  // When ignoring ASTNodes in a scope, they still must count towards a scope's
-  // source range. So include their ranges here
-  SourceRange sourceRangeOfIgnoredASTNodes;
+  mutable Optional<CharSourceRange> cachedCharSourceRange;
 
 #pragma mark - constructor / destructor
 public:
@@ -192,9 +185,6 @@ class ASTScopeImpl {
 public:
   void addChild(ASTScopeImpl *child, ASTContext &);
 
-private:
-  NullablePtr<ASTScopeImpl> getPriorSibling() const;
-
 public:
   void preOrderDo(function_ref<void(ASTScopeImpl *)>);
   /// Like preorderDo but without myself.
@@ -203,78 +193,26 @@ class ASTScopeImpl {
 
 #pragma mark - source ranges
 
-#pragma mark - source range queries
-
 public:
   /// Return signum of ranges. Centralize the invariant that ASTScopes use ends.
   static int compare(SourceRange, SourceRange, const SourceManager &,
                      bool ensureDisjoint);
 
-  SourceRange getSourceRangeOfScope(bool omitAssertions = false) const;
-
-  /// InterpolatedStringLiteralExprs and EditorPlaceHolders respond to
-  /// getSourceRange with the starting point. But we might be asked to lookup an
-  /// identifer within one of them. So, find the real source range of them here.
-  SourceRange getEffectiveSourceRange(ASTNode) const;
-
-  void computeAndCacheSourceRangeOfScope(bool omitAssertions = false) const;
-  bool isSourceRangeCached(bool omitAssertions = false) const;
-
-  bool checkSourceRangeOfThisASTNode() const;
-
-  /// For debugging
-  bool doesRangeMatch(unsigned start, unsigned end, StringRef file = "",
-                      StringRef className = "");
-
-  unsigned countDescendants() const;
-
-  /// Make sure that when the argument is executed, there are as many
-  /// descendants after as before.
-  void assertThatTreeDoesNotShrink(function_ref<void()>);
-
-private:
-  SourceRange computeSourceRangeOfScope(bool omitAssertions = false) const;
-  SourceRange
-  computeSourceRangeOfScopeWithChildASTNodes(bool omitAssertions = false) const;
-  bool ensureNoAncestorsSourceRangeIsCached() const;
-
-#pragma mark - source range adjustments
-private:
-  SourceRange widenSourceRangeForIgnoredASTNodes(SourceRange range) const;
-
-  /// If the scope refers to a Decl whose source range tells the whole story,
-  /// for example a NominalTypeScope, it is not necessary to widen the source
-  /// range by examining the children. In that case we could just return
-  /// the childlessRange here.
-  /// But, we have not marked such scopes yet. Doing so would be an
-  /// optimization.
-  SourceRange widenSourceRangeForChildren(SourceRange range,
-                                          bool omitAssertions) const;
-
-  /// Even ASTNodes that do not form scopes must be included in a Scope's source
-  /// range. Widen the source range of the receiver to include the (ignored)
-  /// node.
-  void widenSourceRangeForIgnoredASTNode(ASTNode);
-
-private:
-  void clearCachedSourceRangesOfMeAndAncestors();
+  CharSourceRange getCharSourceRangeOfScope(SourceManager &SM,
+                                            bool omitAssertions = false) const;
+  bool isCharSourceRangeCached() const;
 
-public: // public for debugging
   /// Returns source range of this node alone, without factoring in any
   /// children.
   virtual SourceRange
   getSourceRangeOfThisASTNode(bool omitAssertions = false) const = 0;
 
 protected:
   SourceManager &getSourceManager() const;
-  bool hasValidSourceRange() const;
-  bool hasValidSourceRangeOfIgnoredASTNodes() const;
-  bool precedesInSource(const ASTScopeImpl *) const;
-  bool verifyThatChildrenAreContainedWithin(SourceRange) const;
-  bool verifyThatThisNodeComeAfterItsPriorSibling() const;
 
 private:
-  bool checkSourceRangeAfterExpansion(const ASTContext &) const;
+  void checkSourceRangeBeforeAddingChild(ASTScopeImpl *child,
+                                         const ASTContext &ctx) const;
 
 #pragma mark common queries
 public:
@@ -329,19 +267,11 @@ class ASTScopeImpl {
   void setWasExpanded() { wasExpanded = true; }
   virtual ASTScopeImpl *expandSpecifically(ScopeCreator &) = 0;
 
-private:
-  /// Compare the pre-expasion range with the post-expansion range and return
-  /// false if lazyiness couild miss lookups.
-  bool checkLazySourceRange(const ASTContext &) const;
-
 public:
   /// Some scopes can be expanded lazily.
-  /// Such scopes must: not change their source ranges after expansion, and
-  /// their expansion must return an insertion point outside themselves.
-  /// After a node is expanded, its source range (getSourceRangeofThisASTNode
-  /// union children's ranges) must be same as this.
+  /// Such scopes must return an insertion point outside themselves when
+  /// expanded.
   virtual NullablePtr<ASTScopeImpl> insertionPointForDeferredExpansion();
-  virtual SourceRange sourceRangeForDeferredExpansion() const;
 
 private:
   virtual ScopeCreator &getScopeCreator();
@@ -545,8 +475,6 @@ class Portion {
 
   virtual NullablePtr<ASTScopeImpl>
   insertionPointForDeferredExpansion(IterableTypeScope *) const = 0;
-  virtual SourceRange
-  sourceRangeForDeferredExpansion(const IterableTypeScope *) const = 0;
   };
 
   // For the whole Decl scope of a GenericType or an Extension
@@ -570,8 +498,6 @@ class Portion {
 
     NullablePtr<ASTScopeImpl>
     insertionPointForDeferredExpansion(IterableTypeScope *) const override;
-    SourceRange
-    sourceRangeForDeferredExpansion(const IterableTypeScope *) const override;
   };
 
   /// GenericTypeOrExtension = GenericType or Extension
@@ -603,8 +529,6 @@ class GenericTypeOrExtensionWherePortion final
 
   NullablePtr<ASTScopeImpl>
   insertionPointForDeferredExpansion(IterableTypeScope *) const override;
-  SourceRange
-  sourceRangeForDeferredExpansion(const IterableTypeScope *) const override;
 };
 
 /// Behavior specific to representing the Body of a NominalTypeDecl or
@@ -622,8 +546,6 @@ class IterableTypeBodyPortion final
 
   NullablePtr<ASTScopeImpl>
   insertionPointForDeferredExpansion(IterableTypeScope *) const override;
-  SourceRange
-  sourceRangeForDeferredExpansion(const IterableTypeScope *) const override;
 };
 
 /// GenericType or Extension scope
@@ -720,7 +642,6 @@ class IterableTypeScope : public GenericTypeScope {
 
 public:
   NullablePtr<ASTScopeImpl> insertionPointForDeferredExpansion() override;
-  SourceRange sourceRangeForDeferredExpansion() const override;
 
   void countBodies(ScopeCreator &) const;
 };
@@ -929,7 +850,6 @@ class FunctionBodyScope : public ASTScopeImpl {
 public:
   std::string getClassName() const override;
   NullablePtr<ASTScopeImpl> insertionPointForDeferredExpansion() override;
-  SourceRange sourceRangeForDeferredExpansion() const override;
 };
 
 class DefaultArgumentInitializerScope final : public ASTScopeImpl {

lib/AST/ASTScope.cpp

@@ -197,23 +197,4 @@ void ASTScopeImpl::postOrderDo(function_ref<void(ASTScopeImpl *)> fn) {
   for (auto *child : getChildren())
     child->postOrderDo(fn);
   fn(this);
-}
-
-unsigned ASTScopeImpl::countDescendants() const {
-  unsigned count = 0;
-  const_cast<ASTScopeImpl *>(this)->preOrderDo(
-      [&](ASTScopeImpl *) { ++count; });
-  return count - 1;
-}
-
-// Can fail if a subscope is lazy and not reexpanded
-void ASTScopeImpl::assertThatTreeDoesNotShrink(function_ref<void()> fn) {
-#ifndef NDEBUG
-  unsigned beforeCount = countDescendants();
-#endif
-  fn();
-#ifndef NDEBUG
-  unsigned afterCount = countDescendants();
-  ASTScopeAssert(beforeCount <= afterCount, "shrank?!");
-#endif
-}
+}

lib/AST/ASTScopeCreation.cpp

@@ -334,8 +334,6 @@ class ScopeCreator final {
 
     ASTScopeImpl *insertionPoint =
         child->expandAndBeCurrentDetectingRecursion(*this);
-    ASTScopeAssert(child->verifyThatThisNodeComeAfterItsPriorSibling(),
-                   "Ensure search will work");
     return insertionPoint;
   }
 
@@ -574,7 +572,6 @@ class NodeAdder
 #define VISIT_AND_IGNORE(What)                                                 \
   NullablePtr<ASTScopeImpl> visit##What(What *w, ASTScopeImpl *p,              \
                                         ScopeCreator &) {                      \
-    p->widenSourceRangeForIgnoredASTNode(w);                                   \
     return p;                                                                  \
   }
 
@@ -768,10 +765,9 @@ class NodeAdder
 
   NullablePtr<ASTScopeImpl> visitExpr(Expr *expr, ASTScopeImpl *p,
                                       ScopeCreator &scopeCreator) {
-    if (expr) {
-      p->widenSourceRangeForIgnoredASTNode(expr);
+    if (expr)
       scopeCreator.addExprToScopeTree(expr, p);
-    }
+
     return p;
   }
 };
@@ -881,6 +877,13 @@ ScopeCreator::addPatternBindingToScopeTree(PatternBindingDecl *patternBinding,
 #pragma mark creation helpers
 
 void ASTScopeImpl::addChild(ASTScopeImpl *child, ASTContext &ctx) {
+  ASTScopeAssert(!child->getParent(), "child should not already have parent");
+  child->parent = this;
+
+#ifndef NDEBUG
+  checkSourceRangeBeforeAddingChild(child, ctx);
+#endif
+
   // If this is the first time we've added children, notify the ASTContext
   // that there's a SmallVector that needs to be cleaned up.
   // FIXME: If we had access to SmallVector::isSmall(), we could do better.
@@ -889,9 +892,6 @@ void ASTScopeImpl::addChild(ASTScopeImpl *child, ASTContext &ctx) {
     haveAddedCleanup = true;
   }
   storedChildren.push_back(child);
-  ASTScopeAssert(!child->getParent(), "child should not already have parent");
-  child->parent = this;
-  clearCachedSourceRangesOfMeAndAncestors();
 }
 
 #pragma mark implementations of expansion
@@ -926,8 +926,6 @@ ASTScopeImpl *ASTScopeImpl::expandAndBeCurrent(ScopeCreator &scopeCreator) {
 
   setWasExpanded();
 
-  ASTScopeAssert(checkSourceRangeAfterExpansion(scopeCreator.getASTContext()),
-                 "Bad range.");
   return insertionPoint;
 }
 

lib/AST/ASTScopeLookup.cpp

@@ -72,22 +72,14 @@ ASTScopeImpl *ASTScopeImpl::findInnermostEnclosingScopeImpl(
                                                       scopeCreator);
 }
 
-bool ASTScopeImpl::checkSourceRangeOfThisASTNode() const {
-  const auto r = getSourceRangeOfThisASTNode();
-  (void)r;
-  ASTScopeAssert(!getSourceManager().isBeforeInBuffer(r.End, r.Start),
-                 "Range is backwards.");
-  return true;
-}
-
 /// If the \p loc is in a new buffer but \p range is not, consider the location
 /// is at the start of replaced range. Otherwise, returns \p loc as is.
 static SourceLoc translateLocForReplacedRange(SourceManager &sourceMgr,
-                                              SourceRange range,
+                                              CharSourceRange range,
                                               SourceLoc loc) {
   if (const auto &replacedRange = sourceMgr.getReplacedRange()) {
     if (sourceMgr.rangeContainsTokenLoc(replacedRange.New, loc) &&
-        !sourceMgr.rangeContains(replacedRange.New, range)) {
+        !sourceMgr.rangeContainsTokenLoc(replacedRange.New, range.getStart())) {
       return replacedRange.Original.Start;
     }
   }
@@ -101,17 +93,19 @@ ASTScopeImpl::findChildContaining(SourceLoc loc,
   auto *const *child = llvm::lower_bound(
       getChildren(), loc,
       [&sourceMgr](const ASTScopeImpl *scope, SourceLoc loc) {
-        ASTScopeAssert(scope->checkSourceRangeOfThisASTNode(), "Bad range.");
-        auto rangeOfScope = scope->getSourceRangeOfScope();
+        auto rangeOfScope = scope->getCharSourceRangeOfScope(sourceMgr);
+        ASTScopeAssert(!sourceMgr.isBeforeInBuffer(rangeOfScope.getEnd(),
+                                                   rangeOfScope.getStart()),
+                       "Source range is backwards");
         loc = translateLocForReplacedRange(sourceMgr, rangeOfScope, loc);
-        return -1 == ASTScopeImpl::compare(rangeOfScope, loc, sourceMgr,
-                                           /*ensureDisjoint=*/false);
+        return (rangeOfScope.getEnd() == loc ||
+                sourceMgr.isBeforeInBuffer(rangeOfScope.getEnd(), loc));
       });
 
   if (child != getChildren().end()) {
-    auto rangeOfScope = (*child)->getSourceRangeOfScope();
+    auto rangeOfScope = (*child)->getCharSourceRangeOfScope(sourceMgr);
     loc = translateLocForReplacedRange(sourceMgr, rangeOfScope, loc);
-    if (sourceMgr.rangeContainsTokenLoc(rangeOfScope, loc))
+    if (rangeOfScope.contains(loc))
       return *child;
   }
 

lib/AST/ASTScopePrinting.cpp

@@ -123,9 +123,7 @@ static void printSourceRange(llvm::raw_ostream &out, const SourceRange range,
 }
 
 void ASTScopeImpl::printRange(llvm::raw_ostream &out) const {
-  if (!isSourceRangeCached(true))
-    out << "(uncached) ";
-  SourceRange range = computeSourceRangeOfScope(/*omitAssertions=*/true);
+  SourceRange range = getSourceRangeOfThisASTNode(/*omitAssertions=*/true);
   printSourceRange(out, range, getSourceManager());
 }