compnerd accepted this revision.
compnerd added a comment.
Some cleanup suggestions included, but I like the change overall.
================
Comment at: lib/Driver/ToolChains/CommonArgs.cpp:1250
+ // Setup statistics file output.
+ if (const Arg *A = Args.getLastArg(options::OPT_save_stats_EQ)) {
+ StringRef SaveStats = A->getValue();
----------------
Early return would be nicer.
```
const Arg *A = Args.getLastArg(options::OPT_save_stats_EQ);
if (!A)
return {};
```
================
Comment at: lib/Driver/ToolChains/CommonArgs.cpp:1264
+ } else {
+ D.Diag(diag::err_drv_invalid_value) << A->getAsString(Args) << SaveStats;
+ }
----------------
Early return after the check would allow you to avoid the boolean and reduce
indentation below too.
```
if (SaveStats != "cwd" && SaveStats != "obj") {
D.Diag(drag::err_drv_invalid_value) << A->getAsString(Args) << SaveStats;
return {};
}
```
================
Comment at: lib/Driver/ToolChains/CommonArgs.h:65
llvm::opt::ArgStringList &CmdArgs, bool IsThinLTO,
const Driver &D);
----------------
While you're here, perhaps you can improve this a slight bit more?
`D` is unnecessary, `ToolChain.getDriver()` allows you to get the value
anyways. I think it would be nicer to move the `Input` and `Output` parameters
to before the `IsThinLTO`.
Repository:
rC Clang
https://reviews.llvm.org/D45771
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
