aganea added a comment.
Thanks for the update @vvereschaka ! Just a few minor things and it should be
good to go.
Also please use `git diff -U99999` next time to add context to the patch.
================
Comment at: clang/docs/UsersManual.rst:783
It is possible to specify this option without any value. In this case
statistics
is printed on standard output in human readable format:
----------------
I think this should read //"In this case statistics **are** printed.."// Would
you mind changing this as well?
================
Comment at: clang/docs/UsersManual.rst:799
+ setting the ``CC_PRINT_PROC_STAT`` and ``CC_PRINT_PROC_STAT_FILE``
environment
+ variables. Use ``CC_PRINT_PROC_STAT_FILE`` to provide a file name to store
+ the statistics.
----------------
Do you think it would be possible to rephrase a bit to avoid the repetition of
`CC_PRINT_PROC_STAT_FILE`? Perhaps also explicitate that `CC_PRINT_PROC_STAT`
is for "enabling the feature and logging to stdout"; while
`CC_PRINT_PROC_STAT_FILE` only "redirects the log to a file".
================
Comment at: clang/lib/Driver/Driver.cpp:1108
+ CCPrintProcessStats = true;
+ Args.ClaimAllArgs(options::OPT_fproc_stat_report);
+
----------------
Remove this too, `hasArg` claims all arguments.
================
Comment at: clang/lib/Driver/Driver.cpp:4039
+
+ if (CCPrintStatReportFilename == nullptr) {
using namespace llvm;
----------------
Suggestion: I find `if (!CCPrintStatReportFilename)` more intutive, more
compact and easier to read, but that's my personal preference. There are
arguments both ways probably, up to you.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97094/new/
https://reviews.llvm.org/D97094
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
