[PATCH] Clang Rename Tool

Manuel Klimek klimek at google.com
Wed Aug 13 11:24:47 PDT 2014


Apart from the nits, I think this now LG :)

================
Comment at: clang-rename/USRFinder.cpp:41
@@ +40,3 @@
+                                      const SourceLocation Point,
+                                      const clang::NamedDecl **Result)
+      : Result(nullptr), SourceMgr(SourceMgr), LangOpts(LangOpts),
----------------
Matthew Plant wrote:
> Manuel Klimek wrote:
> > Use a class member and getter for the result.
> I already did! Funnily enough, you'll notice the Result argument is discarded, and the Result member is a single pointer! I just forgot to remove the constructor argument. oops!
Heh :)

================
Comment at: clang-rename/USRFinder.cpp:132-149
@@ +131,20 @@
+  const auto SearchFile = SourceMgr.getFilename(Point);
+  const NamedDecl *Result;
+
+  NamedDeclFindingASTVisitor Visitor(SourceMgr, LangOpts, Point);
+
+  // We only want to search the decls that exist in the same file as the point.
+  auto Decls = Context.getTranslationUnitDecl()->decls();
+  for (auto &CurrDecl : Decls) {
+    const auto FileLoc = CurrDecl->getLocStart();
+    const auto FileName = SourceMgr.getFilename(FileLoc);
+    if (FileName == SearchFile) {
+      Visitor.TraverseDecl(CurrDecl);
+      Result = Visitor.getNamedDecl();
+      if (Result != nullptr)
+        break;
+    }
+  }
+
+  return Result;
+}
----------------
This now returns an uninitialized pointer if we find nothing... How about:
  for (...) {
    if (...) {
      if (const NamedDecl *Result = Visitor.getNamedDecl())
        return Result;
      }
    }
  }
return nullptr;

================
Comment at: test/clang-rename/VarTest.cpp:1-4
@@ +1,5 @@
+// REQUIRES: shell
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=$(grep -FUbo 'foo;' %t.cpp | head -1 | cut -d: -f1) -new-name=hector %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+namespace A {
----------------
I assume you checked that the test fails if you break the code? :)

http://reviews.llvm.org/D4739






More information about the cfe-commits mailing list