[PATCH] Clang Rename Tool

Manuel Klimek klimek at google.com
Thu Aug 7 02:50:41 PDT 2014


This starts to look really good. Thanks for bearing with me :)

I'd really like to remove the conditional operator renaming for the first version - I think it'll be much easier to follow the code that way (and I don't think it'd be too much work).

For testing, I think there are multiple layers:
1. test that the location finding machinery works - for that I'd actually use unit tests; you can take a look at LocationVerifier in unittest/AST/MatchVerifier
2. test that the renaming works; for that, I think FileChecks are perfect, as you can just
  OldType t;
  // CHECK: NewType t;

================
Comment at: clang-rename/ClangRename.cpp:69-70
@@ +68,4 @@
+newFrontendActionFactory(ActionT *Action) {
+  // Renaming CreateASTConsumer to createASTConsumer doesn't work, so we have to
+  // create our own custom factory.
+  struct SimpleFactory : public tooling::FrontendActionFactory {
----------------
Can you explain what exactly didn't work? Perhaps give me the compile error? But it's no biggie - I can also get rid of this once it's checked in - the template error messages are sometimes hard to get through :)

================
Comment at: clang-rename/ClangRename.cpp:120-133
@@ +119,16 @@
+
+  // Find the USRs.
+  Tool.run(newFrontendActionFactory(&USRAction).get());
+
+  if (USRAction.PrevName.empty())
+    exit(1);
+
+  const auto &USRs = USRAction.USRs;
+  const auto &PrevName = USRAction.PrevName;
+  if (PrintName)
+    errs() << "clang-rename: found name: " << PrevName;
+
+  // Perform the renaming.
+  rename::RenamingAction RenameAction(PrevName, USRs, Tool.getReplacements());
+  auto Factory = newFrontendActionFactory(&RenameAction);
+  int res;
----------------
Nice, this looks much better structured now. Thanks!

http://reviews.llvm.org/D4739






More information about the cfe-commits mailing list