This is extremely useful for language tooling as it allows
for providing go-to-def/find-references/etc. for many
more situations than what is currently possible.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Time | Test | |
---|---|---|
90 ms | x64 debian > Clang.TableGen::redefined-group.td |
Event Timeline
Here is an example screenshot of finding uses of a field in vscode using the TableGen language server:
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.
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.
Aha, I didn't remember that, yes I agree that you cannot unique things if they carry locations. If that is inherent to how tblgen works, then ok.
Would it be possible to do something like unique/have equivalence set representative have union of all locations and unique excluding location?
I haven't looked at innards to see if the above possible, this change doesn't seem too invasive and the results are very useful to produce better error messages too. This is already an improvement in location tracking looking at the changed test results, so LG from me.
llvm/lib/TableGen/TGParser.cpp | ||
---|---|---|
2538 | Why only if singleton? | |
llvm/lib/TableGen/TGParser.h | ||
203 | What does OverrideDefLoc mean? |
What does OverrideDefLoc mean?