jamieschmeiser added a comment.
Good progress.
================
Comment at: clang/lib/Driver/Driver.cpp:97
#include "llvm/Support/raw_ostream.h"
+#include <cassert>
#include <map>
----------------
This include isn't necessary. There are asserts already in the file so this is
transitively included.
================
Comment at: clang/lib/Driver/Driver.cpp:4668
+ // if `-ftime-trace` or `-ftime-trace=<path>` are specified
+ if (TimeTrace || TimeTraceFile) {
+ SmallString<128> TracePath("");
----------------
I think the quick return style is generally preferred. In this case, test the
negative and return. It is easier to follow and limits indenting.
================
Comment at: clang/lib/Driver/Driver.cpp:4674
+ for (auto &J : C.getJobs()) {
+ if (J.getSource().getKind() == Action::LinkJobClass &&
+ !J.getOutputFilenames().empty()) {
----------------
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.
================
Comment at: clang/lib/Driver/Driver.cpp:4679
+ TracePath = llvm::sys::path::parent_path(
+ SmallString<128>(J.getOutputFilenames()[0].c_str()));
+ } else {
----------------
you create the same small string twice. better to create a temporary.
================
Comment at: clang/lib/Driver/Driver.cpp:4689
+ if (J.getSource().getKind() == Action::AssembleJobClass ||
+ J.getSource().getKind() == Action::BackendJobClass) {
+ SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
----------------
Again, test negative with continue
Do you also want CompileJobClass?
================
Comment at: clang/lib/Driver/Driver.cpp:4700
+ llvm::sys::path::append(TracePath,
+ llvm::sys::path::filename(OutputPath));
+ llvm::sys::path::replace_extension(TracePath, "json");
----------------
You are changing TracePath, which will then be altered when you loop back. I
think you need to use a copy of TracePath.
================
Comment at: clang/lib/Driver/Driver.cpp:4714
+ }
+
+ assert(arg.size() > strlen("-ftime-trace") &&
----------------
Can you compute the full path to the directory before the loop and then just
copy it and add the name in the loop for each file?
================
Comment at: clang/lib/Driver/Driver.cpp:4725
+ auto &JArgs = J.getArguments();
+ bool exist = false;
+ for (unsigned I = 0; I < JArgs.size(); ++I) {
----------------
Variables should start with an uppercase letter
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131469/new/
https://reviews.llvm.org/D131469
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
