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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits