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

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Nov 22 11:29:45 2016
New Revision: 287649

URL: http://llvm.org/viewvc/llvm-project?rev=287649&view=rev
Log:
[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.

Modified:
    clang-tools-extra/trunk/clang-rename/USRFinder.cpp
    clang-tools-extra/trunk/test/clang-rename/Variable.cpp

Modified: clang-tools-extra/trunk/clang-rename/USRFinder.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRFinder.cpp?rev=287649&r1=287648&r2=287649&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-rename/USRFinder.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRFinder.cpp Tue Nov 22 11:29:45 2016
@@ -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);
   }
 

Modified: clang-tools-extra/trunk/test/clang-rename/Variable.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-rename/Variable.cpp?rev=287649&r1=287648&r2=287649&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-rename/Variable.cpp (original)
+++ clang-tools-extra/trunk/test/clang-rename/Variable.cpp Tue Nov 22 11:29:45 2016
@@ -1,4 +1,5 @@
-namespace A {
+#define NAMESPACE namespace A
+NAMESPACE {
 int Foo;          /* Test 1 */        // CHECK: int Bar;
 }
 int Foo;                              // CHECK: int Foo;
@@ -20,13 +21,13 @@ void fun() {
 }
 
 // Test 1.
-// RUN: clang-rename -offset=18 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=46 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
 // Test 2.
-// RUN: clang-rename -offset=206 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=234 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
 // Test 3.
-// RUN: clang-rename -offset=613 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=641 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
 // Test 4.
-// RUN: clang-rename -offset=688 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=716 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
 
 // To find offsets after modifying the file, use:
 //   grep -Ubo 'Foo.*' <file>