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