[PATCH] D30760: Record command lines in objects built by clang

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

This is looking pretty good! Aside from a few nits, I'm happy with this after we add tests.



================
Comment at: lib/Driver/ToolChains/Clang.cpp:2725
+  if (Args.hasArg(options::OPT_grecord_gcc_switches)) {
+    std::string CmdLineOpts = "";
+    for (const auto *Arg : Args) CmdLineOpts += Arg->getAsString(Args) + " ";
----------------
Nit: can we use a `SmallString<256>` that we initialize to "-record-cmd-opts=" instead?


================
Comment at: lib/Driver/ToolChains/Clang.cpp:2726
+    std::string CmdLineOpts = "";
+    for (const auto *Arg : Args) CmdLineOpts += Arg->getAsString(Args) + " ";
+    CmdLineOpts.pop_back();
----------------
Style nit: Loop bodies should go on a new line. If you use clang-format, it should handle all of these issues for you. :)


================
Comment at: lib/Driver/ToolChains/Clang.cpp:2728
+    CmdLineOpts.pop_back();
+    CmdArgs.push_back(Args.MakeArgString("-record-cmd-opts=" +
+                                         CmdLineOpts));
----------------
Nit: since we're using a thing directly convertible to StringRef, can we call `MakeArgStringRef` here instead?


https://reviews.llvm.org/D30760