[PATCH] Clang Rename Tool

Manuel Klimek klimek at google.com
Tue Aug 5 00:44:14 PDT 2014


================
Comment at: clang-rename/ClangRename.cpp:159-188
@@ +158,32 @@
+
+    if (PrevName.empty()) {
+      // Retrieve the previous name and USRs.
+      // The source file we are searching will always be the main source file.
+      const auto Point = SourceMgr.getLocForStartOfFile(
+          SourceMgr.getMainFileID()).getLocWithOffset(SymbolOffset);
+      const NamedDecl *TargetDecl = getNamedDeclAt(Context, Point);
+      if (TargetDecl == nullptr) {
+        errs() << "clang-rename: no symbol found at given location\n";
+        *Failed = true;
+        return;
+      }
+
+      // If the decl is in any way related to a class, we want to make sure we
+      // search for the constructor and destructor as well as everything else.
+      if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(TargetDecl))
+        TargetDecl = CtorDecl->getParent();
+      else if (const auto *DtorDecl = dyn_cast<CXXDestructorDecl>(TargetDecl))
+        TargetDecl = DtorDecl->getParent();
+
+      if (const auto *Record = dyn_cast<CXXRecordDecl>(TargetDecl)) {
+        auto ExtraUSRs = getAllConstructorUSRs(Record);
+        USRs.insert(USRs.end(), ExtraUSRs.begin(), ExtraUSRs.end());
+      }
+
+      // Get the primary USR and previous symbol's name.
+      USRs.push_back(getUSRForDecl(TargetDecl));
+      PrevName = TargetDecl->getNameAsString();
+      if (PrintName)
+        errs() << "clang-rename: found symbol: " << PrevName << "\n";
+    }
+
----------------
Matthew Plant wrote:
> Manuel Klimek wrote:
> > As mentioned before, I think we want an extra run for this, so it can be called outside the RenamingASTConsumer. It seems completely unrelated to renaming.
> I agree, but I cannot refactor this to such a degree in such a short amount of time.
If your time does not permit finishing this through, I'd say we stop the review now and start it once somebody is willing to pick up the patch and see it through to the end.

================
Comment at: test/clang-rename/VarTest.cpp:3
@@ +2,3 @@
+namespace A {
+int /*0*/foo;
+}
----------------
Matthew Plant wrote:
> Matthew Plant wrote:
> > Manuel Klimek wrote:
> > > I'm honestly not sure I like this better than:
> > > // CHECK: int bar;
> > This allows us to automatically find the location of the symbol. That way, it checks USRFinder's accuracy as well.
> this lets the tool automatically find locations. I assure you, it is much easier.
But the tests are much harder to read. I'd actually say we want two kinds of tests: one for the USR finder, and one for the rename. The USR finder one might even be a unit test.

http://reviews.llvm.org/D4739






More information about the cfe-commits mailing list