[Lldb-commits] [PATCH] D85705: Add a "Trace" plug-in to LLDB to add process trace support in stages.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I only added a few non-substantial inline comments.



================
Comment at: lldb/include/lldb/Target/Trace.h:39
+public:
+  ~Trace() override = default;
+
----------------
I'm just curious: What's the purpose of this? Is the destructor = 0 in the base class and we want to not have to write this line in Trace implementations?


================
Comment at: lldb/source/Commands/CommandObjectTrace.cpp:88
+    Status error;
+    if (command.size() != 1) {
+      error.SetErrorString("a single path to a JSON file containing trace "
----------------
Would it be possible to early-exit-ify this function? Perhaps by using a lambda for the `result.AppendErrorWithFormat` error return part?


================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:25
+  // Static Functions
+  //------------------------------------------------------------------
+
----------------
FYI: The Doxygen way of writing this is
```
/// Static Functions.
/// \{
  static void Initialize();
  static void Terminate();
/// \}

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705