[PATCH] D150282: [Driver] -ftime-trace: derive trace file names from -o and -dumpdir

[Scott Linder via Phabricator via cfe-commits]

scott.linder added inline comments.


================
Comment at: clang/include/clang/Driver/Compilation.h:279
+  void addTimeTraceFile(const char *Name, const JobAction *JA) {
+    TimeTraceFiles[JA] = Name;
+  }
----------------
MaskRay wrote:
> scott.linder wrote:
> > If this is overriding previous paths should it be called `setTimeTraceFile`?
> The naming is to match `add*File` above.
> Do you want an assert that the entry hasn't been inserted before?
If you think it is useful; otherwise consistency with the other options seems like a good enough reason


================
Comment at: clang/lib/Driver/Driver.cpp:5253
+        Path = DumpDir->getValue();
+        Path += llvm::sys::path::filename(BaseInput);
+      } else {
----------------
MaskRay wrote:
> scott.linder wrote:
> > Why `+=` instead of `append`? Do we just know the value of `dumpdir` is terminated with the path separator?
> `-dumpdir ` is a bit misnomer that it may or may not end with a path separator.
> 
> `clang -c -ftime-trace d/a.c -o e/xa.o -dumpdir f/` is intended to create `fa.json`
> 
> I updated a test and the description to give an example.
Thanks, I just forgot this point since the previous discussions! LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150282