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

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 13 22:18:10 PDT 2022


MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Driver/Driver.cpp:4516
+  bool HasTimeTrace = C.getArgs().hasArg(options::OPT_ftime_trace);
+  bool HasTimeTraceFile = C.getArgs().hasArg(options::OPT_ftime_trace_EQ);
+  // Whether `-ftime-trace` or `-ftime-trace=<path>` are specified
----------------
Change this variable to use `getLastArg(options::OPT_ftime_trace_EQ)` instead.

The convention is to use getLastArg if both hasArg and getLastArg are needed.


================
Comment at: clang/lib/Driver/Driver.cpp:4517
+  bool HasTimeTraceFile = C.getArgs().hasArg(options::OPT_ftime_trace_EQ);
+  // Whether `-ftime-trace` or `-ftime-trace=<path>` are specified
+  if (!HasTimeTrace && !HasTimeTraceFile)
----------------
Delete the comment. Don't add comment which just repeats what simply code does.


================
Comment at: clang/lib/Driver/Driver.cpp:4526
+  //         then the full OutputFile's path may be appended to it.
+  SmallString<128> TracePath("");
+  if (HasTimeTraceFile) {
----------------



================
Comment at: clang/lib/Driver/Driver.cpp:4528
+  if (HasTimeTraceFile) {
+    TracePath = SmallString<128>(
+        C.getArgs().getLastArg(options::OPT_ftime_trace_EQ)->getValue());
----------------
Delete `SmallString<128>` => `TracePath = ...`


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