[PATCH] D24192: [clang-refactor] introducing clang-refactor

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: clang-refactor/driver/Driver.cpp:41
@@ +40,3 @@
+    Command = argv[1];
+    std::string Invocation = std::string(argv[0]) + " " + argv[1];
+    argv[1] = Invocation.c_str();
----------------
curdeius wrote:
> Simpler:
> 
> ```
> std::string Invocation = argv[0] + (" " + Command);
> ```
  (llvm::Twine(argv[0]) + " " + ...).str()

================
Comment at: clang-refactor/driver/ModuleManager.h:21
@@ +20,3 @@
+
+using namespace llvm;
+
----------------
curdeius wrote:
> `using namespace` in header?!
If you need StringRef, ArrayRef etc. in clang namespace, just include "clang/Basic/LLVM.h".

================
Comment at: clang-refactor/modules/core/RefactoringModule.h:91
@@ +90,3 @@
+  //
+  int run(int argc, const char **argv) {
+    // Register options.
----------------
I believe, `run` should be external to the module. Its implementation might be different depending on whether we're just running all stages sequentially or trying to parallelize processing of different translation units.

================
Comment at: clang-refactor/modules/core/RefactoringModule.h:148
@@ +147,3 @@
+  // Panic: if there are conflicting replacements.
+  virtual int applyReplacementsOrOutputDiagnostics() = 0;
+
----------------
This should be possible to implement in a refactoring-independent way in clang-refactor itself. Or do you see any issues with this?


https://reviews.llvm.org/D24192