[PATCH] Clang Rename Tool

[Manuel Klimek]

I think there are basically 2 blockers before this can go in:
1. Make the tests actually test something :)
2. Use a member/getter for the result (see inline comment)

Otherwise LG for a first landing.

================
Comment at: clang-rename/ClangRename.cpp:16-42
@@ +15,29 @@
+
+#include "USRFindingAction.h"
+#include "RenamingAction.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/Frontend/CommandLineSourceLoc.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Parse/Parser.h"
+#include "clang/Parse/ParseAST.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/Host.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include <ctype.h>
+#include <string>
+#include <vector>
+
----------------
Matthew Plant wrote:
> Manuel Klimek wrote:
> > Are all of these still needed?
> Not entirely sure. The only real way to determine is to remove one at a time, or manually go through and determine if each is actually used.
> Isn't there a clang tool for this? remove-unused-headers or something?
Not yet :) Patches welcome!

================
Comment at: clang-rename/ClangRename.cpp:141-145
@@ +140,7 @@
+  const auto &USRs = USRAction.getUSRs();
+  const auto &PrevName = USRAction.getUSRSpelling();
+
+  if (PrevName.empty())
+    // An error should have already been printed.
+    exit(1);
+
----------------
We really need to convert this to Diagnostics (fine in a follow-up patch though).

================
Comment at: clang-rename/USRFinder.cpp:41
@@ +40,3 @@
+                                      const SourceLocation Point,
+                                      const clang::NamedDecl **Result)
+      : Result(nullptr), SourceMgr(SourceMgr), LangOpts(LangOpts),
----------------
Use a class member and getter for the result.

================
Comment at: clang-rename/USRLocFinder.cpp:43-47
@@ +42,7 @@
+    if (getUSRForDecl(Decl) == USR) {
+      // Because we traverse the AST from top to bottom, it follows that the
+      // current locations must be further down (or right) than the previous one
+      // inspected. This has the effect of keeping LocationsFound sorted by
+      // line first column second with no effort of our own.
+      // This is correct as of 2014-06-27
+      LocationsFound.push_back(Decl->getLocation());
----------------
This somehow sneaked back in?

================
Comment at: test/clang-rename/VarTest.cpp:3-8
@@ +2,8 @@
+// RUN: clang-rename -offset=126 -new-name=hector %t.cpp -- | FileCheck %s < %t.cpp
+namespace A {
+int foo;  // CHECK: int hector;
+}
+int foo;  // CHECK: int foo;
+int bar = foo; // CHECK: bar = foo;
+int baz = A::foo; // CHECK: baz = A::hector;
+void fun1() {
----------------
Matthew Plant wrote:
> Manuel Klimek wrote:
> > Thanks! This is much easier for me to read :)
> No problem, but boy was that offset hard to find :P
Ah, we need some adjustments:
1. Add // REQUIRES: shell at the top
2. copying the temp file only makes sense if we do in-place replacement; which I think we should do
3. The CHECK comments get matched by the CHECKS. You'll want:
sed 's,//.*,,' %t.cpp | FileCheck %s
4. Isn't that offset wrong? Both vim and
$ grep -FUbo 'foo;' /tmp/test
tell me the offset is 145. So, why not instead do
-offset=$(grep -FUbo 'foo;' %t.cpp |head -1 |cut -d: -f1)


================
Comment at: unittests/clang-rename/USRLocFindingTest.cpp:77
@@ +76,3 @@
+TEST(USRLocFinding, FindsVarUSR) {
+  const char VarTest[] = R"(
+namespace A {
----------------
I'm so so so sorry, but I lied about raw strings :'( - apparently VS 2012 doesn't support them yet, so they're not allowed in clang :( :( :(

http://reviews.llvm.org/D4739