This is an archive of the discontinued LLVM Phabricator instance.

[clangAST] support TypeLoc in TextNodeDumper
Needs ReviewPublic

Authored by upsj on Oct 8 2022, 12:37 PM.

Details

Reviewers
aaron.ballman
Summary

TextNodeDumper currently doesn't support printing the TypeLoc hierarchy, which makes navigating clang-query output involving TypeLoc hard to understand.
This diff replicates the Type hierarchy as well as possible, relying on Type visitors for printing type-specific info.
The only exception where this doesn't work is most sugared types and any kinds of parameters (function or template), for which the TypeLoc is not easily accessible.

Example clang-query output:

clang-query> set output dump
clang-query> match typeLoc(loc(type().bind("type")))
...
Binding for "root":
QualifiedTypeLoc </home/tribizel/llvm-project/build/test.cpp:23:10, col:60> 'matrix::Dense<typename remove_complex_t<T>::type> *const' const
`-PointerTypeLoc <col:10, col:60> 'matrix::Dense<typename remove_complex_t<T>::type> *'
  `-ElaboratedTypeLoc <col:10, col:58> 'matrix::Dense<typename remove_complex_t<T>::type>'
    `-TemplateSpecializationTypeLoc <col:18, col:58> 'Dense<typename remove_complex_t<T>::type>' Dense

Binding for "type":
PointerType 0x261f2b0 'matrix::Dense<typename remove_complex_t<T>::type> *' dependent
`-ElaboratedType 0x261f250 'matrix::Dense<typename remove_complex_t<T>::type>' sugar dependent
  `-TemplateSpecializationType 0x261f210 'Dense<typename remove_complex_t<T>::type>' dependent Dense
    `-TemplateArgument type 'typename remove_complex_t<T>::type'
      `-DependentNameType 0x261f190 'typename remove_complex_t<T>::type' dependent

Diff Detail

Event Timeline

upsj created this revision.Oct 8 2022, 12:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 8 2022, 12:37 PM
upsj requested review of this revision.Oct 8 2022, 12:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 8 2022, 12:37 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Thank you for working on this! Should we make similar changes to the JSONNodeDumper as well? (It doesn't seem to have TypeLoc support.)

clang/include/clang/AST/ASTDumperUtils.h
55–56 ↗(On Diff #466304)

I don't know that we need a color specific to type locs; those should behave the same as a type node (after all, it's a type coupled with a source location, basically).

clang/include/clang/AST/TextNodeDumper.h
337

Should you also handle IncompleteArrayTypeLoc as well?

clang/lib/AST/TextNodeDumper.cpp
1643

These uses of dyn_cast can all be switched to cast instead because we already know the type we're going to find.

1652

This one is actually not quite right. You need to go through the ASTContext to get to an array type. This should use ASTContext::getAsConstantArrayType() instead.

1657

Similarly, ASTContext::getAsVariableArrayType()

1663

ASTContext::getAsDependentSizedArrayType()

clang/lib/AST/TypeLoc.cpp
70–72

It might make sense for this to return a StringRef instead so it's clear that the caller does not own the memory or have to worry about releasing it. WDYT?

upsj updated this revision to Diff 467698.Oct 14 2022, 12:19 AM
upsj marked 6 inline comments as done.

I incorporated the cast, ASTContext and StringRef suggestions. About JSONDumper, I was surprised to see that it doesn't seem to use AddChild for the AST hierarchy at all, so I'm not quite clear how (if?) it builds a nested output. What's the easiest way to invoke it?

clang/include/clang/AST/TextNodeDumper.h
337

Not sure about this one, since it has no additional metadata - the type location visitor already prints out the type class name

upsj added a comment.Oct 14 2022, 1:15 AM

Let me backtrack my previous question: I misremembered where AddChild is actually being called in the traversal (ASTNodeTraverser, not TextNodeDumper), so I'll see if adding this to the JSON output is feasible