[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

Jamie Schmeiser via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 22 07:03:44 PDT 2022


jamieschmeiser added inline comments.


================
Comment at: clang/lib/Driver/Driver.cpp:4667
+  // Whether `-ftime-trace` or `-ftime-trace=<path>` are specified
+  if (!TimeTrace && !TimeTraceFile) return;
+
----------------
The return should be on the next line.  Did you run this through clang-format?  Is it okay with this on the same line?


================
Comment at: clang/lib/Driver/Driver.cpp:4674
+    for (auto &J : C.getJobs()) {
+      if (J.getSource().getKind() == Action::LinkJobClass &&
+          !J.getOutputFilenames().empty()) {
----------------
dongjunduo wrote:
> jamieschmeiser wrote:
> > Can you have a link job without an output filename?  If not, then just have an assert for !empty.  Again, the negative test and continue might be easier to understand.
> The output filename should not be empty. 
> 
> If the "-o" is not specified, the output filename may be "a.out".
Yes.  If the output filename should not be empty, then assert that rather than testing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469



More information about the cfe-commits mailing list