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

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added inline comments.


================
Comment at: clang/lib/Driver/Driver.cpp:1107
+    CCPrintProcessStats = true;
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report);
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report_EQ);
----------------
Please remove, `getLastArg` already claims all arguments for this option.


================
Comment at: clang/lib/Driver/Driver.cpp:4031
+
+      // Use FinalOutput variable to get the output file name.
+      const char *LinkingOutput = nullptr;
----------------
I think the code below is safe-explanatory, I don't think you need this comment, it doesn't add much value.


================
Comment at: clang/lib/Driver/Driver.cpp:4050
                << ", mem=" << ProcStat->PeakMemory << " Kb\n";
-      }
-      if (!StatReportFile.empty()) {
+      } else {
         // CSV format.
----------------
The previous behavior was printing both the output and the CSV file when specifying `-fproc-stat-report -fproc-stat-report=file.csv`, now it would only emit the CSV, is that intended? @sepavloff what do you think?


================
Comment at: clang/test/Driver/cc-print-proc-stat.c:1
+// RUN: env CC_PRINT_PROC_STAT=1 \
+// RUN:     CC_PRINT_PROC_STAT_FILE=%t.csv \
----------------
Could you please expand a bit the test to entirely cover all the codepaths above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97094