omtcyfz added inline comments.
================
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());
----------------
alexfh wrote:
> nit: No need for braces around single-line `if` bodies.
Well, I'm really happy with braces everywhere. The codestyle doesn't prohibit
it and I think it's more readable than no braces around single statements in
logical blocks.
================
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 {
----------------
alexfh wrote:
> We should implement it then ;)
Yes, it's in my TODO list. But I suppose it's better to fix clang-rename first
and get back to it a bit later.
As we discussed, there will probably be no need in RecursiveASTVisitors at all
in the end.
Anyway, the patch is already in the state when nobody wants to review it :D I'd
be happy just to land it and to continue with all the other bugs present in the
tool.
================
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 --
----------------
alexfh wrote:
> I guess, this test will need `// REQUIRES: shell`. You can try to commit
> without it, but watch windows buildbots closely.
This test? There's `RUN: cat %s > %t.cpp` in every single one of them, I'm not
sure what's the issue.
https://reviews.llvm.org/D22465
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
