[clang-rename] Prune away AST nodes more correctly and effectively when looking for a point

Due to the way the preprocessor works nodes can be half in a macro or a
different file. This means checking the file name of the start location
of a Decl is not a correct way of checking if the entire Decl is in that
file. Remove that flawed assumption and replace it with a more effective
check: If the point we're looking for is *inside* of the begin and end
location of a Decl, look inside.

This should make clang-rename more reliable (for example macro'd namespaces
threw it off before, as seen in the test case) and maybe a little faster
by pruning off more stuff that the RecursiveASTVisitor doesn't have to
drill into.

llvm-svn: 287649

1 files changed, 9 insertions, 6 deletions

diff --git a/clang-tools-extra/clang-rename/USRFinder.cpp b/clang-tools-extra/clang-rename/USRFinder.cpp
index 494bc75b56b..7733c6f088e 100644
--- a/clang-tools-extra/clang-rename/USRFinder.cpp
+++ b/clang-tools-extra/clang-rename/USRFinder.cpp
@@ -168,15 +168,18 @@ private:
 
 const NamedDecl *getNamedDeclAt(const ASTContext &Context,
                                 const SourceLocation Point) {
-  StringRef SearchFile = Context.getSourceManager().getFilename(Point);
+  const SourceManager &SM = Context.getSourceManager();
   NamedDeclFindingASTVisitor Visitor(Point, Context);
 
-  // We only want to search the decls that exist in the same file as the point.
+  // Try to be clever about pruning down the number of top-level declarations we
+  // see. If both start and end is either before or after the point we're
+  // looking for the point cannot be inside of this decl. Don't even look at it.
   for (auto *CurrDecl : Context.getTranslationUnitDecl()->decls()) {
-    const SourceLocation FileLoc = CurrDecl->getLocStart();
-    StringRef FileName = Context.getSourceManager().getFilename(FileLoc);
-    // FIXME: Add test.
-    if (FileName == SearchFile)
+    SourceLocation StartLoc = CurrDecl->getLocStart();
+    SourceLocation EndLoc = CurrDecl->getLocEnd();
+    if (StartLoc.isValid() && EndLoc.isValid() &&
+        SM.isBeforeInTranslationUnit(StartLoc, Point) !=
+            SM.isBeforeInTranslationUnit(EndLoc, Point))
       Visitor.TraverseDecl(CurrDecl);
   }