[PATCH] D90221: Include more attribute details when dumping AST in JSON

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.



In D90221#2358469 <https://reviews.llvm.org/D90221#2358469>, @aronsky wrote:

> In D90221#2357060 <https://reviews.llvm.org/D90221#2357060>, @aaron.ballman wrote:
>
>> In D90221#2356110 <https://reviews.llvm.org/D90221#2356110>, @aronsky wrote:
>>
>>> In D90221#2356062 <https://reviews.llvm.org/D90221#2356062>, @lebedev.ri wrote:
>>>
>>>> Are there tests missing?
>>>
>>> Quite possible. I followed the trail of the existing functions to figure out the difference between JSON and textual dumping, and tried replicating everything in a manner similar to the existing code. I haven't run into any tests, but that's probably because I wasn't looking for those. I'll add the appropriate tests ASAP.
>>
>> FWIW, the tests for dumping JSON live in `clang\test\AST` and typically have a `-json` extension on them. There is a helper script named `gen_ast_dump_json_test.py` that can be used to generate the expected output from the test.
>
> Thanks, I'll take a look at the Python script, that'll be helpful!
>
> Those functions do look out of place, but they are actually called via polymorphism (I wish I could point to the exact location - it wasn't easy figuring that out in the first place, and the actual work was done about a month ago, I just got to publishing the PR yesterday). The code that calls these functions is emitted at `writeDump` (in `clang/utils/TableGen/ClangAttrEmitter.cpp`) - which, in turn, is called by `EmitClangAttrJSONNodeDump` and `EmitClangAttrTextNodeDump` in the same file.

It seems surprising to me that you'd have to add those declarations; I think that's a layering violation. There's something somewhat strange going on here that I think is related. Given:

  [[clang::annotate("testing")]] void func1();
  [[gnu::visibility("hidden")]] void func2();

I get:

  {
    "id": "0x2527bc0",
    "kind": "FunctionDecl",
    "loc": {
      "offset": 36,
      "file": "F:\\Aaron Ballman\\Desktop\\test.cpp",
      "line": 1,
      "col": 37,
      "tokLen": 5
    },
    "range": {
      "begin": {
        "offset": 31,
        "col": 32,
        "tokLen": 4
      },
      "end": {
        "offset": 42,
        "col": 43,
        "tokLen": 1
      }
    },
    "name": "func1",
    "mangledName": "?func1@@YAXXZ",
    "type": {
      "qualType": "void ()"
    },
    "inner": [
      {
        "id": "0x2527c60",
        "kind": "AnnotateAttr",
        "range": {
          "begin": {
            "offset": 2,
            "col": 3,
            "tokLen": 5
          },
          "end": {
            "offset": 27,
            "col": 28,
            "tokLen": 1
          }
        },
        "args": [
          " \"testing\""
        ],
        "inner": [
          {
            "id": "0x24bcfd8",
            "kind": "StringLiteral",
            "range": {
              "begin": {
                "offset": 18,
                "col": 19,
                "tokLen": 9
              },
              "end": {
                "offset": 18,
                "col": 19,
                "tokLen": 9
              }
            },
            "type": {
              "qualType": "const char [8]"
            },
            "valueCategory": "lvalue",
            "value": "\"testing\""
          }
        ]
      }
    ]
  },
  {
    "id": "0x2527de8",
    "kind": "FunctionDecl",
    "loc": {
      "offset": 81,
      "line": 2,
      "col": 36,
      "tokLen": 5
    },
    "range": {
      "begin": {
        "offset": 76,
        "col": 31,
        "tokLen": 4
      },
      "end": {
        "offset": 87,
        "col": 42,
        "tokLen": 1
      }
    },
    "name": "func2",
    "mangledName": "?func2@@YAXXZ",
    "type": {
      "qualType": "void ()"
    },
    "inner": [
      {
        "id": "0x2527e88",
        "kind": "VisibilityAttr",
        "range": {
          "begin": {
            "offset": 48,
            "col": 3,
            "tokLen": 3
          },
          "end": {
            "offset": 72,
            "col": 27,
            "tokLen": 1
          }
        },
        "args": [
          " Hidden"
        ]
      }
    ]
  }

Notice how the annotate attribute has an `inner` array with the argument inside of it while the visibility attribute does not? Ideally, we'd either use `args` or `inner` but not a mixture of the two.

I was imagining the design here to be that the JSON node dumper for attributes would open a new JSON attribute named "args" whose value is an array of arguments, then you'd loop over the arguments and `Visit` each one in turn as the arguments are children of the attribute AST node in some respects. For instance, consider dumping the arguments for the `enable_if` attribute, which accepts arbitrary expressions -- dumping a type or a bare decl ref is insufficient.



================
Comment at: clang/test/AST/ast-dump-stmt-json.cpp:1465
+// CHECK-NEXT:    "args": [
+// CHECK-NEXT:     " Default"
+// CHECK-NEXT:    ]
----------------
There's extra leading whitespace here that seems like it shouldn't be there.


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

https://reviews.llvm.org/D90221