[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

Vlad Vereschaka via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 25 13:38:16 PST 2021


vvereschaka added inline comments.


================
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.
----------------
aganea wrote:
> 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".
Yes, sure. Here is updated and extended version of the doc part.


================
Comment at: clang/lib/Driver/Driver.cpp:4039
+
+      if (CCPrintStatReportFilename == nullptr) {
         using namespace llvm;
----------------
aganea wrote:
> 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.
I have updated it in according of you suggestion. It is ok for me too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97094/new/

https://reviews.llvm.org/D97094



More information about the cfe-commits mailing list