[PATCH] Clang Rename Tool

[Manuel Klimek]

First round focused on ClangRename.cpp. More to come :)

================
Comment at: clang-rename/ClangRename.cpp:69-71
@@ +68,5 @@
+            cl::cat(ClangRenameCategory));
+static cl::opt<std::string> SymbolLocation(cl::Positional,
+                                           cl::desc("<file>:<line>:<column>"),
+                                           cl::cat(ClangRenameCategory));
+static cl::list<std::string> FileNames(cl::Positional, cl::desc("[<file> ...]"),
----------------
I usually find offsets better to use for tools (and humans are not going to enter lines and columns here anyway).

================
Comment at: clang-rename/ClangRename.cpp:82
@@ +81,3 @@
+      (FileName == "-" ? "<stdin>" : FileName), Source->getBufferSize(), 0);
+  Sources.overrideFileContents(Entry, Source, true);
+  return Sources.createFileID(Entry, SourceLocation(), SrcMgr::C_User);
----------------
This is also already a feature of ClangTool / RefactoringTool if I'm not mistaken.

================
Comment at: clang-rename/ClangRename.cpp:90
@@ +89,3 @@
+// the input was given by a Clang getNameAsString method.
+static std::string getOpCallShortForm(const std::string &Call) {
+  const std::string Prefix = "operator";
----------------
I'd try to not use getNameAsString to do matching against - can't we solve this in the type system?

================
Comment at: clang-rename/ClangRename.cpp:100
@@ +99,3 @@
+// Get the USRs for the constructors of the class.
+static std::vector<std::string> getClassEquivUSRs(const CXXRecordDecl *Decl) {
+  std::vector<std::string> USRs;
----------------
Name this getAllConstructorUSRs?

================
Comment at: clang-rename/ClangRename.cpp:113
@@ +112,3 @@
+  // explicit calls to a destructor through TagTypeLoc (and it is better for the
+  // purpose of renaming).
+  //
----------------
I wonder whether we could do something similar for constructors (perhaps by changing the AST).

================
Comment at: clang-rename/ClangRename.cpp:125-128
@@ +124,6 @@
+
+static std::vector<SourceLocation>
+fixOperatorLocs(const std::string &OpSpelling,
+                const std::vector<SourceLocation> &OldLocs,
+                const SourceManager &SourceMgr, const LangOptions &LangOpts) {
+  std::vector<SourceLocation> Candidates;
----------------
Needs a comment.

================
Comment at: clang-rename/ClangRename.cpp:131
@@ +130,3 @@
+  for (auto Loc : OldLocs) {
+    // If the current location poihnts to the "operator" keyword, we want to
+    // skip to the actual operator's spelling. We don't try to do this very; if
----------------
points

================
Comment at: clang-rename/ClangRename.cpp:131-133
@@ +130,5 @@
+  for (auto Loc : OldLocs) {
+    // If the current location poihnts to the "operator" keyword, we want to
+    // skip to the actual operator's spelling. We don't try to do this very; if
+    // the operator isn't the next token over, we give up on the location.
+    SmallVector<char, 32> Buffer;
----------------
Manuel Klimek wrote:
> points
Renaming operators seems ... strange; I would expect to add that (if at all) somewhen much later.

================
Comment at: clang-rename/ClangRename.cpp:169
@@ +168,3 @@
+
+    if (PrevName.empty()) {
+      // Retrieve the previous name and USRs.
----------------
I find this rather strange architecturally - why don't we just visit the whole thing twice?

================
Comment at: clang-rename/ClangRename.cpp:173
@@ +172,3 @@
+          ParsedSourceLocation::FromString(SymbolLocation);
+      // The source file we are searching will always be the mail source file.
+      const auto Point = SourceMgr.translateLineCol(
----------------
s/mail/main/

================
Comment at: clang-rename/ClangRename.cpp:233
@@ +232,3 @@
+
+    // ...and we're done!
+  }
----------------
Remove comment.

================
Comment at: clang-rename/ClangRename.cpp:248-258
@@ +247,13 @@
+static bool Rename() {
+  CompilerInstance Compiler;
+
+  auto *Invocation = new CompilerInvocation;
+  Compiler.setInvocation(Invocation);
+
+  // Make sure we're parsing the right language.
+  Compiler.getLangOpts().CPlusPlus11 = true;
+  Compiler.getLangOpts().GNUKeywords = true;
+  Compiler.getLangOpts().NoBuiltin = false;
+  Invocation->setLangDefaults(Compiler.getLangOpts(), IK_CXX,
+                              LangStandard::lang_gnucxx11);
+
----------------
Any reason you're not using RefactoringTool (which already does basically everything that's in this file)?

http://reviews.llvm.org/D4739