[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