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

Fri Aug 28 02:25:54 PDT 2020

labath added a comment.

In D85705#2243536 <https://reviews.llvm.org/D85705#2243536>, @wallace wrote:

> - I'm still using StructuredData for the parsing. There's a chance that once we release this feature users will want support for other formats besides JSON, so for now I prefer to ask for JSON input but parse in a format-agnostic way. If eventually no one needs any other format, we can switch to JSON-only parsing.

That argument goes both ways. We could say that if anyone wants to add a non-json format, he can change the code to accept his preferred format. Overall, I am very sceptical of the StructuredData's ability to abstract diverse formats in a meaningful way. I doubt that a natural XML representation of this data would map to the same abstract format as this json and that it could be parsed by the same code -- simply because xml conventions are different, and xml has features that json doesn't (attributes, for one).

And since this file is just a descriptor, and actual plugin-specific data is contained in other files, I don't see why it's format couldn't be fixed.

================
Comment at: lldb/test/API/commands/trace/intelpt-trace/trace.json:11
+  },
+  "triple": "x86_64-*-linux",
+  "processes": [
----------------
What if one of the processes is 64-bit and the other 32? It sounds like this should be a property of a particular process.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85705