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

Tareq A. Siraj tareq.a.siraj at intel.com
Fri Oct 4 09:08:37 PDT 2013


  LGTM with some minor comments.


================
Comment at: clang-modernize/Core/ReplacementHandling.cpp:90
@@ +89,3 @@
+bool ReplacementHandling::applyReplacements() {
+  SmallVector<const char *, 16> Argv;
+  Argv.push_back(CARPath.c_str());
----------------
Do we need a initial length of 16? Looks like we are only using 7 and unlikely this will change soon. Suggest setting it to 8 for now.

================
Comment at: clang-modernize/Core/ReplacementHandling.cpp:108
@@ +107,3 @@
+  bool ExecutionFailed = false;
+  int ReturnCode = ExecuteAndWait(CARPath.c_str(), Argv.data(), /* env */ 0,
+                                  /* redirects */ 0,
----------------
Wondering if we should pass the environment of current process to the child to be safe. Does it use any pre-processing stuff? The pre-processor I think uses CPATH and some other environment vars.


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



More information about the cfe-commits mailing list