On Tue, 2025-05-06 at 11:34 +0100, Richard Sandiford wrote:
> <soum...@nvidia.com> writes:
> > From: Soumya AR <soum...@nvidia.com>
> >
> > Hi,
> >
> > This RFC and subsequent patch series introduces support for
> > printing and parsing
> > of aarch64 tuning parameters in the form of JSON.
>
> Thanks for doing this. It looks really useful. My main question is:
> rather than write the parsing and printing routines by hand, could we
> generate the structure definitions, the parsing code, and the
> printing
> code from the schema?
>
> The schema would need to provide more information about the
> structures
> compared to the current one. The approach would also presumably need
> build/*.o versions of the json routines. But it seems like something
> that we might want to do elsewhere, so would be worth building a bit
> of infrastructure around. And it would reduce the maintenance burden
> associated with adding a new field or changing an existing one.
FWIW I have a lot of similar written-by-hand JSON-handling code for
SARIF; see gcc/libsarifreplay.cc for decoding from json and
gcc/diagnostic-format-sarif.cc for generating json. It would be nice
to have some tooling for this based on schema files - but there are
lots of awkward cases to cope with - SARIF is non-trivial [1]
Other approaches which I've dabbled with are:
* what I called strongly-typed json, where as well as json::object and
json::array that can take arbitrary json::value as property values or
elements, to have a way to support objects where known properties are
stored in a special way (strongly typed); similarly for arrays.
* to have an adapter for json-like types, so that given a tree-like
structure in memory you can go direct to json output without needing to
build an in-memory tree; similar for parsing (iirc llvm has something
like this)
Note that for SARIF the DejaGnu tests validate the generated json
against a schema; see gcc/testsuite/lib/scansarif.exp; there's also
run-sarif-pytest which allows DejaGnu to run Python scripts to verify
properties of the generated json. This is probably overkill for the
aarch64 tuning use-case, but is very helpful for SARIF, which is deeply
nested json, has cross-references, etc.
Dave
[1] Another wart here is that libsarifreplay.cc is built on top of
libgdiagnostics.h rather than diagnostic-core.h. One nice things I
have is reporting chapter-and-verse of the specification when the
schema is violated, along with both logical location (JSON pointer) and
physical location, e.g.:
In JSON property '/runs/0/results/0/level':
bad-level.sarif:12:20: error: unrecognized value for 'level': 'mostly harmless'
[SARIF v2.1.0 §3.27.10]
12 | "level": "mostly harmless",
| ^~~~~~~~~~~~~~~~~
Dave
