On Thu, 2023-08-03 at 15:54 +0100, Matthew Malcomson wrote: > On 8/3/23 15:09, David Malcolm wrote: > > > > Hi Matthew. I recently touched the timevar code (in r14-2881- > > g75d623946d4b6e) to add support for serializing the timevar data in > > JSON form as part of the SARIF output (PR analyzer/109361). > > > > Looking at your patch, it looks like the baseline for the patch > > seems > > to predate r14-2881-g75d623946d4b6e. > > > > I don't have a strong opinion on the implementation choices in your > > patch, but please can you rebase to beyond my recent change and > > make > > sure that the SARIF serialization still works with your patch. > > > > Specifically, please try compiling with > > -ftime-report -fdiagnostics-format=sarif-file > > and have a look at the generated .sarif file, e.g. via > > python -m json.tool foo.c.sarif > > which will pretty-print the JSON to stdout. > > > > Currently I'm writing out the values as floating-point seconds, and > > AFAIK my analyzer integration testsuite [1] is the only consumer of > > this data. > > Hi David, > > Thanks for the heads-up. Will update the patch. > > I read your last paragraph as suggesting that you'd be open to > changing > the format. Is that correct?
I suppose, but I'd prefer to keep the existing format. > > I would initially assume that writing out the time as floating-point > seconds would still be most convenient for your use since it looks to > be > like something to be presented to a person. Yes. I may be biased in that with -fanalyzer the times tend to be measured in seconds rather than fractions of seconds, alas. > > However, since I don't know much about the intended uses of SARIF in > general I figured I should double-check -- does that choice to remain > printing out floating-point seconds seem best to you? I'd prefer to keep the JSON output as floating-point seconds, if that's not too much of a pain. Dave > > > > > [...snip...] > > > > Thanks > > Dave > > [1] > > https://github.com/davidmalcolm/gcc-analyzer-integration-tests/issues/5 > > >
