[PATCH] D134087: [TableGen] Track reference locations of Records/RecordVals

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 17 13:40:22 PDT 2022


rriddle added a comment.

In D134087#3797799 <https://reviews.llvm.org/D134087#3797799>, @rriddle wrote:

> In D134087#3797798 <https://reviews.llvm.org/D134087#3797798>, @lattner wrote:
>
>> Very cool, it'd be great to improve this in tblgen!
>>
>> I'm a long way from working on this code, but the representation is a bit odd.  Typically you'd put a location on `VarInit` when forming the `VarInit::get`.  Having a location on this would enable far better diagnostics across all of tblgen.
>>
>> That said, it would make the jump to definition implementation slightly more awkward, because it has to scan the AST to find references, instead of having them on the declaration.  I'm concerned that taking a step like in this patch would be a sideways step for improving tblgen though.
>
> The problem that I ran into was that a lot of stuff kind of requires that the Inits get folded and are uniqued, which would no longer be the case if we put locations on them. From there this patch arose, because I didn't want to broach the "rewrite the world" just to get better locations.

I suppose framed in another way, I view this patch as a pragmatic step to enabling better tooling without fundamentally restructuring how tablegen works internally. I would love to just rewrite TableGen to actually work like a real programming language, but the cost of that is quite high. I also
don't think this patch is that high of a maintenance burden even if we want to do things in a better way eventually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134087



More information about the llvm-commits mailing list