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>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
