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