[PATCH] clang-modernize: Apply replacements using clang-apply-replacements

[Guillaume Papin]

  Hope you didn't commit already. One of my comment is worth being corrected I think, other are potential style issues.


================
Comment at: clang-modernize/Core/ReplacementHandling.h:109
@@ +108,3 @@
+  /// \param[out] Resulting name is placed here.
+  static void generateTempDir(llvm::SmallVectorImpl<char> &Result);
+
----------------
This could return a `std::string` to make the interface easier.

================
Comment at: clang-modernize/Core/ReplacementHandling.h:113-117
@@ +112,7 @@
+
+  llvm::SmallString<128> CARPath;
+  llvm::SmallString<128> DestinationDir;
+  bool DoFormat;
+  llvm::SmallString<8> FormatStyle;
+  llvm::SmallString<128> StyleConfigDir;
+};
----------------
I don't think it's wrong per se but I think using more than one SmallString/Vector as class field should be avoided. Maybe this class will never be used on the heap but it seems okay to me to have `std::string` here.

================
Comment at: clang-modernize/Core/ReplacementHandling.h:41
@@ +40,3 @@
+  /// \param[in] Dir Destination directory  name
+  void setDestinationDir(const llvm::StringRef Dir) { DestinationDir = Dir; }
+
----------------
[personal_opinion]
I see a lot of const `llvm::StringRef`, IMHO StringRefs have the same "value semantic" as primitive types. In the same way I don't use const for integers I won't use it here.
[/personal_opinion]

================
Comment at: clang-modernize/Core/ReplacementHandling.cpp:32-33
@@ +31,4 @@
+
+  void *symbol = (void *)(intptr_t) & generateTempDir;
+  CARPath = fs::getMainExecutable(Argv0, symbol);
+  StringRef ParentPath = path::parent_path(CARPath);
----------------
Not sure but I think the space after `&` isn't needed.

Looking at `fs::getMainExecutable()` examples I saw the following that seems easier to read:
```
static int staticSymbol;
std::string mainExecutable = llvm::sys::fs::getMainExecutable("oclint", &staticSymbol);
```

================
Comment at: clang-modernize/Core/ReplacementHandling.cpp:34-35
@@ +33,4 @@
+  CARPath = fs::getMainExecutable(Argv0, symbol);
+  StringRef ParentPath = path::parent_path(CARPath);
+  SmallString<128> TestPath(ParentPath);
+  path::append(TestPath, "clang-apply-replacements");
----------------
Maybe not possible but seems like one line can be as clear as those 2 lines:
```
SmallString<128> TestPath = path::parent_path(CARPath);
```

================
Comment at: clang-modernize/Core/ReplacementHandling.cpp:147
@@ +146,3 @@
+          fs::createUniqueFile(Prefix + "_%%_%%_%%_%%_%%_%%.yaml", Result)) {
+    Error.append(EC.message().begin(), EC.message().end());
+    return false;
----------------
`error_code::message()` returns a new string object. Calling `begin()/end()` will be from different objects potentially no?



http://llvm-reviews.chandlerc.com/D1836

BRANCH
  use-car

ARCANIST PROJECT
  clang-tools-extra