[PATCH] D137088: [readobj] Give JSON output a custom printSymbol

[Paul Kirth via Phabricator via llvm-commits]

paulkirth added a comment.

Sorry, I typed this up back in early November, when we were initially looking at this and forgot to hit the submit button. I've had a few other items take up my attention, but hope to spend a bit more time on this before the end of the year.

The point of this change is to make Other a consistent field, and avoid having to special case how its handled when Other is 0.

  Other: 0,   ->   "Other": {
                     "RawFlags": 0,
                      "Flags": []
                   },

With more context the JSON looks like this:

- Before

  ... 
   "Symbols": [
      {
        "Symbol": {
          "Name": {
            "Value": "foo",
            "RawValue": 1
          },
          "Value": 0,
          "Size": 0,
          "Binding": {
            "Value": "Local",
            "RawValue": 0
          },
          "Type": {
            "Value": "None",
            "RawValue": 0
          },
          "Other": 0,
          "Section": {
            "Value": ".text",
            "RawValue": 1
          }
        }
      }
    ]
    ...



- After

  ...
  "Symbols": [
     {
       "Symbol": {
         "Name": {
           "Value": "foo",
           "RawValue": 1
         },
         "Value": 0,
         "Size": 0,
         "Binding": {
           "Value": "Local",
           "RawValue": 0
         },
         "Type": {
           "Value": "None",
           "RawValue": 0
         },
         "Other": {
           "RawFlags": 0,
           "Flags": []
         },
         "Section": {
           "Value": ".text",
           "RawValue": 1
         }
       }
     }
   ]
   ...

It isn't significantly different than before, but now it's //consistent//, regardless of the flag's value.  For tools that focuses on a human reader, the specializing the output to be concise makes sense. But JSON is a machine readable format, so it's far more important that the output always have the same structure than it is to be concise. That's really the point of this change.

Also, part of the reason I set these patches aside for a while was that there is little testing for the JSON output, so I was trying to find a middle ground where we could only test that the formatting is correct without duplicating many of the LLVM formating tests. I didn't really find a nice way to do that. Do you have any recommendations as to how we could achieve that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137088