[PATCH] D137091: [llvm-readobj] Rename JSON Flag fields to be more consistent

Tue Mar 21 11:31:03 PDT 2023

paulkirth added a comment.

In D137091#4209313 <https://reviews.llvm.org/D137091#4209313>, @hans wrote:

> This broke some Chromium builds: https://bugs.chromium.org/p/chromium/issues/detail?id=1426287
>
> It turns out we have a tool that consumes this JSON. Maybe that one can be made to accept the encoding both before and after this change, but there may be others.
>
> That raises the question of what the compatibility story is here. Are we okay with breaking all consumers of this JSON?

Unfortunately, I think we kind of need to be OK with that here. The fact is the JSON output should probably have been considered experimental, or kept behind a hidden flag until it had been vetted a bit more.

Prior to my patches, almost nothing out of `llvm-readobj` was valid JSON.  Portions of it would be OK(sometimes even large portions), but there were many changes we needed to make to have the output be considered valid, and usable by existing tools like `jq`. It wasn't so bad that you couldn't work around it, but it was pretty painful. This particular renaming could arguably have been avoided, but it does make it possible to process all "flag" types uniformly, which didn't happen under the old schema. Since we were already breaking everything else, this seems to be a good time to make a change like that. I don't foresee the need to do more updates like this, where we modify the schema.

Given the trouble I've had using the JSON output programmatically, I doubt there are many consumers. I think it's also likely that anyone affected would appreciate the tool emitting proper JSON that any off-the-shelf parser can consume, so that they can ditch the custom logic required to use the JSON emitted by `llvm`readobj`.

I do dislike that this is probably breaking downstream consumers though. I don't know what else we can do since we're fixing a fundamental problem.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137091