[PATCH] D22465: [clang-rename] introduce better symbol finding and add few more tests

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 21 18:35:33 PDT 2016


alexfh requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: clang-rename/USRFinder.cpp:81
@@ +80,3 @@
+    const auto TypeEndLoc = Loc.getEndLoc(),
+               TypeBeginLoc = Lexer::GetBeginningOfToken(
+                   TypeEndLoc, SourceMgr, Context->getLangOpts());
----------------
Declarations of multiple variables can be confusing. Please split this declaration.

================
Comment at: clang-rename/USRFinder.cpp:94
@@ -91,4 +93,3 @@
       const auto *Decl = NameLoc.getNestedNameSpecifier()->getAsNamespace();
-      if (Decl && !setResult(Decl, NameLoc.getLocalBeginLoc(),
-                             Decl->getNameAsString().length()))
-        return false;
+      if (Decl) {
+        setResult(Decl, NameLoc.getLocalBeginLoc(), NameLoc.getLocalEndLoc());
----------------
nit: No need for braces around single-line `if` bodies.

================
Comment at: clang-rename/USRFinder.h:49
@@ +48,3 @@
+// FIXME: This wouldn't be needed if
+// RecursiveASTVisitor<T>::VisitNestedNameSpecifier would have been implemented.
+class NestedNameSpecifierLocFinder : public MatchFinder::MatchCallback {
----------------
We should implement it then ;)

================
Comment at: test/clang-rename/ComplicatedClassType.cpp:1
@@ -1,2 +1,2 @@
-// Unsupported test.
 // RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=220 -new-name=Bar %t.cpp -i --
----------------
I guess, this test will need `// REQUIRES: shell`. You can try to commit without it, but watch windows buildbots closely.


https://reviews.llvm.org/D22465





More information about the cfe-commits mailing list