Merge pull request #28039 from davidungar/factor-ASTScope-ordering

[NameLookup ASTScope] Factor ASTScope ordering

Author: David Ungar

include/swift/AST/ASTScope.h

@@ -226,6 +226,10 @@ class ASTScopeImpl {
 #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

lib/AST/ASTScopeCreation.cpp

@@ -52,7 +52,9 @@ static bool rangeableIsIgnored(const Stmt *d) {
   return false; // ??
 }
 static bool rangeableIsIgnored(const ASTNode n) {
-  return n.is<Decl *>() && n.get<Decl *>()->isImplicit();
+  return (n.is<Decl *>() && rangeableIsIgnored(n.get<Decl *>())) ||
+         (n.is<Stmt *>() && rangeableIsIgnored(n.get<Stmt *>())) ||
+         (n.is<Expr *>() && rangeableIsIgnored(n.get<Expr *>()));
 }
 
 template <typename Rangeable>
@@ -600,25 +602,12 @@ class ScopeCreator final {
 
   template <typename Rangeable>
   bool isNotAfter(Rangeable n1, Rangeable n2) const {
-    auto cmpLoc = [&](const SourceLoc l1, const SourceLoc l2) {
-      return l1 == l2 ? 0 : ctx.SourceMgr.isBeforeInBuffer(l1, l2) ? -1 : 1;
-    };
     const auto r1 = getRangeableSourceRange(n1);
     const auto r2 = getRangeableSourceRange(n2);
-    const int startOrder = cmpLoc(r1.Start, r2.Start);
-    const int endOrder = cmpLoc(r1.End, r2.End);
-
-#ifndef NDEBUG
-    if (startOrder * endOrder == -1) {
-      llvm::errs() << "*** Start order contradicts end order between: ***\n";
-      dumpRangeable(n1, llvm::errs());
-      llvm::errs() << "\n*** and: ***\n";
-      dumpRangeable(n2, llvm::errs());
-    }
-#endif
-    ASTScopeAssert(startOrder * endOrder != -1,
-                   "Start order contradicts end order");
-    return startOrder + endOrder < 1;
+
+    const int signum = ASTScopeImpl::compare(r1, r2, ctx.SourceMgr,
+                                             /*ensureDisjoint=*/true);
+    return -1 == signum;
   }
 
   static bool isVarDeclInPatternBindingDecl(ASTNode n1, ASTNode n2) {

lib/AST/ASTScopeLookup.cpp

@@ -152,13 +152,16 @@ ASTScopeImpl::findChildContaining(SourceLoc loc,
 
     bool operator()(const ASTScopeImpl *scope, SourceLoc loc) {
       ASTScopeAssert(scope->checkSourceRangeOfThisASTNode(), "Bad range.");
-      return sourceMgr.isBeforeInBuffer(scope->getSourceRangeOfScope().End,
-                                        loc);
+      return -1 == ASTScopeImpl::compare(scope->getSourceRangeOfScope(), loc,
+                                         sourceMgr,
+                                         /*ensureDisjoint=*/false);
     }
     bool operator()(SourceLoc loc, const ASTScopeImpl *scope) {
       ASTScopeAssert(scope->checkSourceRangeOfThisASTNode(), "Bad range.");
-      return !sourceMgr.isBeforeInBuffer(scope->getSourceRangeOfScope().End,
-                                         loc);
+      // Alternatively, we could check that loc < start-of-scope
+      return 0 >= ASTScopeImpl::compare(loc, scope->getSourceRangeOfScope(),
+                                        sourceMgr,
+                                        /*ensureDisjoint=*/false);
     }
   };
   auto *const *child = std::lower_bound(

lib/AST/ASTScopeSourceRange.cpp

@@ -764,3 +764,34 @@ SourceLoc ast_scope::extractNearestSourceLoc(
   const ASTScopeImpl *scope = std::get<0>(scopeAndCreator);
   return scope->getSourceRangeOfThisASTNode().Start;
 }
+
+int ASTScopeImpl::compare(const SourceRange lhs, const SourceRange rhs,
+                          const SourceManager &SM, const bool ensureDisjoint) {
+  ASTScopeAssert(!SM.isBeforeInBuffer(lhs.End, lhs.Start),
+                 "Range is backwards.");
+  ASTScopeAssert(!SM.isBeforeInBuffer(rhs.End, rhs.Start),
+                 "Range is backwards.");
+
+  auto cmpLoc = [&](const SourceLoc lhs, const SourceLoc rhs) {
+    return lhs == rhs ? 0 : SM.isBeforeInBuffer(lhs, rhs) ? -1 : 1;
+  };
+  // Establish that we use end locations throughout ASTScopes here
+  const int endOrder = cmpLoc(lhs.End, rhs.End);
+
+#ifndef NDEBUG
+  if (ensureDisjoint) {
+    const int startOrder = cmpLoc(lhs.Start, rhs.Start);
+
+    if (startOrder * endOrder == -1) {
+      llvm::errs() << "*** Start order contradicts end order between: ***\n";
+      lhs.print(llvm::errs(), SM, false);
+      llvm::errs() << "\n*** and: ***\n";
+      rhs.print(llvm::errs(), SM, false);
+    }
+    ASTScopeAssert(startOrder * endOrder != -1,
+                   "Start order contradicts end order");
+  }
+#endif
+
+  return endOrder;
+}