[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