Modify lldb-test to print out ASTs from symbol file

[shafik]

Summary

Currently when invoking lldb-test symbols -dump-ast it parses all the debug symbols and calls print(...) on the TranslationUnitDecl.

While useful the TranslationUnitDecl::print(...) method gives us a higher level view then the dump from ASTDumper which is what we get when we invoke dump() on a specific AST node.

The main motivation for this change is allow us to verify that the AST nodes we create when we parse DWARF. For example in order to verify we are correctly using DIFlagExportSymbols added by D66667 we would want to dump a CXXRecordDecl:

CXXRecordDecl 0x7fdc578295f8 <<invalid sloc>> <invalid sloc> struct definition
|-DefinitionData is_anonymous pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
...

and check for the is_anonymous flag or the lack thereof.

My current approach is to enumerate the symbols and to obtain specific AST nodes if possible and dump them. It looks in some cases such as FunctionDecls we don't have direct access to it but I don't think that is an major problem at this point.

I did not implement it but we should probably have a way of request a specific type instead of indiscriminately dumping all of them.

This is a work in progress and I am not sure if the following is the best approach and I am open to alternative approaches or modification to the current one.

[shafik]

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3057

I added this change because currently when end up trying to parse non-types which generates diagnostic like the following:

error: anon.o {0x0000001e}: unhandled type tag 0x0034 (DW_TAG_variable), please file a bug and attach the file at the start of this error message

[xiaobai]

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3029

nit: stray whitespace

3032

Why is this now two lines?

tools/lldb-test/lldb-test.cpp

551–579

nit: clang-format this block, some of the lines are long and somewhat difficult to read on phabricator.

[JDevlieghere]

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3029

Trailing whitespace.

3030

I assume this is unintentional?

3057

Assuming these are LLVM enum values, there's a method in Dwarf.h named isType which tells you if a given tag is a type. This list is missing at least some DWARF5 types (DW_TAG_dynamic_type, DW_TAG_atomic_type...).

tools/lldb-test/lldb-test.cpp

553

The formatting seems off. Did you run this through clang-format?

561

Personally I think this would be a lot more readable with braces.

[shafik]

• Formatting code
• Removing FunctionDecl case since it was not correct

[clayborg]

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3032

indent is wrong

3058–3072

We should add a "IsType()" to DWARFDie here. This list can get out of date as new type tags are added. There are a few other places that could use the "bool DWARFDie::ItType() const" function too. Or we should just fix ParseType by adding an option to it like an extra bool like "bool warnIfNotType" so you don't get the "error:" message.

tools/lldb-test/lldb-test.cpp

552–579

I know that there is already clang AST stuff in this file, but it seems like anything in this file should be just passed to the type system for dumping? This code could be:

lldb_private::TypeList type_list;
size_t ntypes = symfile->GetTypes(nullptr, eTypeClassAny, type_list);
printf( "Type list size: %zu\n", ntypes);
for( size_t i = 0; i < ntypes; ++i)
  type_list.GetTypeAtIndex(i)->DumpTypeValue(...);

Better yet this entire function could just become:

Error opts::symbols::dumpAST(lldb_private::Module &Module) {
  Module.ParseAllDebugSymbols();

  auto symfile = Module.GetSymbolFile();
  if (!symfile)
    return make_string_error("Module has no symbol file.");

  auto type_system_or_err =
      symfile->GetTypeSystemForLanguage(eLanguageTypeC_plus_plus);
  if (type_system_or_err)
    type_system_or_err->DumpAST(...);
  else
    return make_string_error("Can't retrieve TypeSystem");
}

And all clang AST specific stuff can be removed from this binary? Tests would need to be updated.

[aprantl]

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3074

You also probably don't want to actually print anything here?

tools/lldb-test/lldb-test.cpp

555

The rest of the file seems to use llvm::outs() instead of printf.

564

clang-format

570

switch over CompilerType->getAsClangType() ->getKind()?

[labath]

For dumping a specific type something like this could be right, but for "indiscriminately dumping" everything, this seems to be a bit fragile.

Would it be possible to make this use the SymbolFile::DumpClangAST method (this is what the "image dump ast" lldb command uses), and then possibly change that method to include any extra information you need? I already see the DefinitionData line when i do a "image dump clang ast", so it's possible you wouldn't need to change anything there...

FTR, this is what i get from image dump ast:

Dumping clang ast for 1 modules.
TranslationUnitDecl 0x561fa5fd9128 <<invalid sloc>> <invalid sloc> <undeserialized declarations>
`-NamespaceDecl 0x561fa5fd99e8 <<invalid sloc>> <invalid sloc> Q
  `-CXXRecordDecl 0x561fa5fd9a70 <<invalid sloc>> <invalid sloc> struct A definition
    |-DefinitionData pass_in_registers empty standard_layout trivially_copyable has_user_declared_ctor can_const_default_init
    | |-DefaultConstructor exists non_trivial user_provided defaulted_is_constexpr
    | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
    | |-MoveConstructor exists simple trivial needs_implicit
    | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
    | |-MoveAssignment exists simple trivial needs_implicit
    | `-Destructor simple irrelevant trivial needs_implicit
    `-CXXConstructorDecl 0x561fa5fd9bf0 <<invalid sloc>> <invalid sloc> A 'void ()'

[shafik]

In D67994#1682051, @labath wrote:

For dumping a specific type something like this could be right, but for "indiscriminately dumping" everything, this seems to be a bit fragile.

Would it be possible to make this use the SymbolFile::DumpClangAST method (this is what the "image dump ast" lldb command uses), and then possibly change that method to include any extra information you need? I already see the DefinitionData line when i do a "image dump clang ast", so it's possible you wouldn't need to change anything there...

FTR, this is what i get from image dump ast:

Dumping clang ast for 1 modules.
TranslationUnitDecl 0x561fa5fd9128 <<invalid sloc>> <invalid sloc> <undeserialized declarations>
`-NamespaceDecl 0x561fa5fd99e8 <<invalid sloc>> <invalid sloc> Q
  `-CXXRecordDecl 0x561fa5fd9a70 <<invalid sloc>> <invalid sloc> struct A definition
    |-DefinitionData pass_in_registers empty standard_layout trivially_copyable has_user_declared_ctor can_const_default_init
    | |-DefaultConstructor exists non_trivial user_provided defaulted_is_constexpr
    | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
    | |-MoveConstructor exists simple trivial needs_implicit
    | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
    | |-MoveAssignment exists simple trivial needs_implicit
    | `-Destructor simple irrelevant trivial needs_implicit
    `-CXXConstructorDecl 0x561fa5fd9bf0 <<invalid sloc>> <invalid sloc> A 'void ()'

Using this approach for this simple example:

using ULongArrayTypedef = unsigned long[10];

int main() {
    ULongArrayTypedef *p;
}

I only obtain:

TranslationUnitDecl 0x7fd5eb023608 <<invalid sloc>> <invalid sloc> <undeserialized declarations>

Vs this output for what I have now:

ULongArrayTypedef *
PointerType 0x7fea0a825080 'unsigned long (*)[10]'
`-ConstantArrayType 0x7fea0a824ed0 'unsigned long [10]' 10 
  `-BuiltinType 0x7fea0a8247c0 'unsigned long'
int
BuiltinType 0x7fea0a824700 'int'
ULongArrayTypedef
TypedefDecl 0x7fea0a825000 <<invalid sloc>> <invalid sloc> ULongArrayTypedef 'unsigned long [10]'
`-ConstantArrayType 0x7fea0a824ed0 'unsigned long [10]' 10 
  `-BuiltinType 0x7fea0a8247c0 'unsigned long'
long unsigned int
BuiltinType 0x7fea0a8247c0 'unsigned long'
unsigned long [10]
ConstantArrayType 0x7fea0a824ed0 'unsigned long [10]' 10 
`-BuiltinType 0x7fea0a8247c0 'unsigned long'
main
FunctionProtoType 0x7fea0a824f10 'int (void)' cdecl
`-BuiltinType 0x7fea0a824700 'int'

I believe this is due to us being lazy as to when we import.

[shafik]

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3074

Apologies, I did not clean up all the these up.

[shafik]

tools/lldb-test/lldb-test.cpp

552–579

If we do stick with this approach pushing the DumpAST(...) into TypeSystem seems reasonable.

[labath]

In D67994#1683440, @shafik wrote:

I believe this is due to us being lazy as to when we import.

Yes, but doesn't calling Module::ParseAllDebugSymbols force us to parse everything? "image dump ast" does dump only the things that have been parsed, but that doesn't mean it lldb-test needs to do that too.

IOW, I was not saying you should use "image dump ast" to write the test you wanted to write. I was merely saying that we should try to make "lldb-test -dump-ast" use the same dumping code as "image dump ast" does (assuming the latter outputs the kind of data that you need, but it seems to me that it does...). "image dump ast" can remain lazy, and only show the things that have been parsed so far (which is also useful sometimes), while "lldb-test" can do whatever it takes to parse everything (I would hope that is merely calling Module::ParseAllDebugSymbols).

[shafik]

• Updated approach based on comment
• Pushed clang ast manipulation into ClangASTContext.cpp
• Created a new option in lldb-test to keep the new functionality separate from the old use called dumpClangAST
• Cleaned up checking DWARF type by using isType()

[shafik]

In D67994#1683934, @labath wrote:

In D67994#1683440, @shafik wrote:

I believe this is due to us being lazy as to when we import.

Yes, but doesn't calling Module::ParseAllDebugSymbols force us to parse everything? "image dump ast" does dump only the things that have been parsed, but that doesn't mean it lldb-test needs to do that too.

IOW, I was not saying you should use "image dump ast" to write the test you wanted to write. I was merely saying that we should try to make "lldb-test -dump-ast" use the same dumping code as "image dump ast" does (assuming the latter outputs the kind of data that you need, but it seems to me that it does...). "image dump ast" can remain lazy, and only show the things that have been parsed so far (which is also useful sometimes), while "lldb-test" can do whatever it takes to parse everything (I would hope that is merely calling Module::ParseAllDebugSymbols).

In order to reuse the functionality we would need to import the complete clang ASTs to the scratch AST context and that would end up being a lot more work then the current approach. So I stuck with the current approach.

[shafik]

I ended up splitting out the functionality into a new method dumpClangAST it looks PDB is not as lazy as DWARF and there are several PDB tests already using the current dumpAST as it is.

tools/lldb-test/lldb-test.cpp

552–579

It looked like pushing this into ClangASTContext made more sense. I am happy to bikeshed naming though.

[aprantl]

include/lldb/Symbol/ClangASTContext.h

895 ↗

(On Diff #222861)

nit: . at the end

896 ↗

(On Diff #222861)

What is "the symbol table" in this context? Does ClangASTContext have access to one?

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3082

comment explaining why not subrange type?

source/Symbol/ClangASTContext.cpp

9166 ↗

(On Diff #222861)

Ah .. so should it be called DumpFromSymbolFile() or LookupAndDump()?

9173 ↗

(On Diff #222861)

This API with the return value doesn't exit any more.

9182 ↗

(On Diff #222861)

the AsCString() part seems to be bad for the performance.. is there a variant that returns a StringRef instead?

9184 ↗

(On Diff #222861)

A switch over the kind would be more efficient and potentially nicer to read?

9195 ↗

(On Diff #222861)

is an EnumDecl not a TagDecl?

[shafik]

Updates based on comments:

• Remove use of AsCString()
• Fixed use of old API
• other minor issues

[labath]

Maybe this is my fault since I'm the one who introduced the first bunch of arguments here IIRC, but anyway, I have a feeling all of these dumping options are getting out of hand. Looking at the argument list, you'd expect that -dump-ast, and -dump-clang-ast do something completely different, but they actually print the exact same AST, only one allows you to print a subset of it, while the other one doesn't.

Given that the ast dumping is currently incompatible with all/most of the other dumping options anyway, maybe we should turn over a clean slate, and implement a new subcommand specifically dedicated to dumping clang ast (as opposed to the internal lldb representations of types/functions/..., which is what the "symbols" subcommand started out as, and which is what most of its options are dedicated to).

OR, and this would-be super cool, if it was actually possible, we could try to make ast-dumping compatible with the existing searching options. Currently, if you type lldb-test symbols -find=type -name=foo it will search for types named "foo", and print some summary of the lldb object representing that type. What if we made it so that adding -dump-ast to that command caused the tool to dump the *AST* of the types it has found (e.g., in addition to the previous summary)? I think that would make the behavior of the tool most natural to a new user, but I am not sure whether that would actually fit in with your goals here...

[labath]

In D67994#1691641, @shafik wrote:

In D67994#1683934, @labath wrote:

IOW, I was not saying you should use "image dump ast" to write the test you wanted to write. I was merely saying that we should try to make "lldb-test -dump-ast" use the same dumping code as "image dump ast" does (assuming the latter outputs the kind of data that you need, but it seems to me that it does...). "image dump ast" can remain lazy, and only show the things that have been parsed so far (which is also useful sometimes), while "lldb-test" can do whatever it takes to parse everything (I would hope that is merely calling Module::ParseAllDebugSymbols).

In order to reuse the functionality we would need to import the complete clang ASTs to the scratch AST context and that would end up being a lot more work then the current approach. So I stuck with the current approach.

I am sorry, all of these ast contexts are still fairly confusing to me, so I am not sure what this actually means. Can you elaborate?

[shafik]

In D67994#1692645, @labath wrote:

Maybe this is my fault since I'm the one who introduced the first bunch of arguments here IIRC, but anyway, I have a feeling all of these dumping options are getting out of hand. Looking at the argument list, you'd expect that -dump-ast, and -dump-clang-ast do something completely different, but they actually print the exact same AST, only one allows you to print a subset of it, while the other one doesn't.

Given that the ast dumping is currently incompatible with all/most of the other dumping options anyway, maybe we should turn over a clean slate, and implement a new subcommand specifically dedicated to dumping clang ast (as opposed to the internal lldb representations of types/functions/..., which is what the "symbols" subcommand started out as, and which is what most of its options are dedicated to).

OR, and this would-be super cool, if it was actually possible, we could try to make ast-dumping compatible with the existing searching options. Currently, if you type lldb-test symbols -find=type -name=foo it will search for types named "foo", and print some summary of the lldb object representing that type. What if we made it so that adding -dump-ast to that command caused the tool to dump the *AST* of the types it has found (e.g., in addition to the previous summary)? I think that would make the behavior of the tool most natural to a new user, but I am not sure whether that would actually fit in with your goals here...

The output is actually quite different for example given:

struct A {
  struct {
      int x;
  };
} a;

the output of lldb-test symbols -dump-ast anon.o is:

Module: anon.o
struct A;

while the output of lldb-test symbols -dump-clang-ast anon.o is:

Module: anon.o
A
CXXRecordDecl 0x7ff3698262c8 <<invalid sloc>> <invalid sloc> struct A definition
|-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
| |-DefaultConstructor exists trivial needs_implicit
| |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| |-MoveConstructor exists simple trivial needs_implicit
| |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| |-MoveAssignment exists simple trivial needs_implicit
| `-Destructor simple irrelevant trivial needs_implicit
|-CXXRecordDecl 0x7ff3698263f8 <<invalid sloc>> <invalid sloc> struct definition
| |-DefinitionData is_anonymous pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
| | |-DefaultConstructor exists trivial needs_implicit
| | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveConstructor exists simple trivial needs_implicit
| | |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveAssignment exists simple trivial needs_implicit
| | `-Destructor simple irrelevant trivial needs_implicit
| `-FieldDecl 0x7ff369826528 <<invalid sloc>> <invalid sloc> x 'int'
|-FieldDecl 0x7ff369826578 <<invalid sloc>> <invalid sloc> implicit 'A::(anonymous struct)'
`-IndirectFieldDecl 0x7ff3698265f0 <<invalid sloc>> <invalid sloc> implicit x 'int'
  |-Field 0x7ff369826578 '' 'A::(anonymous struct)'
  `-Field 0x7ff369826528 'x' 'int'
A::(anonymous struct)
CXXRecordDecl 0x7ff3698263f8 <<invalid sloc>> <invalid sloc> struct definition
|-DefinitionData is_anonymous pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
| |-DefaultConstructor exists trivial needs_implicit
| |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| |-MoveConstructor exists simple trivial needs_implicit
| |-CopyAssignment trivial has_const_param needs_implicit implicit_has_const_param
| |-MoveAssignment exists simple trivial needs_implicit
| `-Destructor simple irrelevant trivial needs_implicit
`-FieldDecl 0x7ff369826528 <<invalid sloc>> <invalid sloc> x 'int'
int
BuiltinType 0x7ff369825b00 'int'

Given they are both working with the Symbol File I believe they both belong under the symbol command. I feel like -dump-ast is a little misleading but -dump-clang-ast feels good.

include/lldb/Symbol/ClangASTContext.h

896 ↗

(On Diff #222861)

It comes from`GetSymbolFile() which is inherited from TypeSystem`

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

3082

Good catch! Currently ParseTypeFromDWARF(...) which is called by ParseType(...) does not handle DW_TAG_subrange_type and it is not clear ATM if this is a bug or not.

source/Symbol/ClangASTContext.cpp

9184 ↗

(On Diff #222861)

That would be nice but I don't think there is a way to get to the type what will allow me to use a switch.

9195 ↗

(On Diff #222861)

I don't understand the comment, TagDecl is handled above.

[teemperor]

source/Symbol/ClangASTContext.cpp

9195 ↗

(On Diff #222861)

I think Adrian's point is that EnumDecl is a subclass of TagDecl.

[labath]

In D67994#1695172, @shafik wrote:

In D67994#1692645, @labath wrote:

Maybe this is my fault since I'm the one who introduced the first bunch of arguments here IIRC, but anyway, I have a feeling all of these dumping options are getting out of hand. Looking at the argument list, you'd expect that -dump-ast, and -dump-clang-ast do something completely different, but they actually print the exact same AST, only one allows you to print a subset of it, while the other one doesn't.

Given that the ast dumping is currently incompatible with all/most of the other dumping options anyway, maybe we should turn over a clean slate, and implement a new subcommand specifically dedicated to dumping clang ast (as opposed to the internal lldb representations of types/functions/..., which is what the "symbols" subcommand started out as, and which is what most of its options are dedicated to).

OR, and this would-be super cool, if it was actually possible, we could try to make ast-dumping compatible with the existing searching options. Currently, if you type lldb-test symbols -find=type -name=foo it will search for types named "foo", and print some summary of the lldb object representing that type. What if we made it so that adding -dump-ast to that command caused the tool to dump the *AST* of the types it has found (e.g., in addition to the previous summary)? I think that would make the behavior of the tool most natural to a new user, but I am not sure whether that would actually fit in with your goals here...

The output is actually quite different for example given:

struct A {
  struct {
      int x;
  };
} a;

the output of lldb-test symbols -dump-ast anon.o is:

Module: anon.o
struct A;

while the output of lldb-test symbols -dump-clang-ast anon.o is:

Module: anon.o
A
CXXRecordDecl 0x7ff3698262c8 <<invalid sloc>> <invalid sloc> struct A definition
|-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
...

Given they are both working with the Symbol File I believe they both belong under the symbol command. I feel like -dump-ast is a little misleading but -dump-clang-ast feels good.

You're right about. I'm sorry, I should have checked what -dump-ast does. It definitely is confusing that it does something completely different than the "dump ast" lldb command.

But what about the second part of my comment? I think it would still be nice if the lldb-test symbols options could be somehow factored into two groups:

• a group which selects *what* to dump: this would be -find, -name, etc.
• a group which selects *how* to dump it: -dump-clang-ast, -dump-ast :/, the default mode

would it be somehow possible to hook in this logic into the findTypes function in lldb-test, so that instead of calling TypeMap::Dump, it does something else if -dump-clang-ast flag is present (this "else" resuling in the printing of clang ast)?

[shafik]

In D67994#1697981, @labath wrote:

In D67994#1695172, @shafik wrote:

In D67994#1692645, @labath wrote:

Maybe this is my fault since I'm the one who introduced the first bunch of arguments here IIRC, but anyway, I have a feeling all of these dumping options are getting out of hand. Looking at the argument list, you'd expect that -dump-ast, and -dump-clang-ast do something completely different, but they actually print the exact same AST, only one allows you to print a subset of it, while the other one doesn't.

Given that the ast dumping is currently incompatible with all/most of the other dumping options anyway, maybe we should turn over a clean slate, and implement a new subcommand specifically dedicated to dumping clang ast (as opposed to the internal lldb representations of types/functions/..., which is what the "symbols" subcommand started out as, and which is what most of its options are dedicated to).

OR, and this would-be super cool, if it was actually possible, we could try to make ast-dumping compatible with the existing searching options. Currently, if you type lldb-test symbols -find=type -name=foo it will search for types named "foo", and print some summary of the lldb object representing that type. What if we made it so that adding -dump-ast to that command caused the tool to dump the *AST* of the types it has found (e.g., in addition to the previous summary)? I think that would make the behavior of the tool most natural to a new user, but I am not sure whether that would actually fit in with your goals here...

The output is actually quite different for example given:

struct A {
  struct {
      int x;
  };
} a;

the output of lldb-test symbols -dump-ast anon.o is:

Module: anon.o
struct A;

while the output of lldb-test symbols -dump-clang-ast anon.o is:

Module: anon.o
A
CXXRecordDecl 0x7ff3698262c8 <<invalid sloc>> <invalid sloc> struct A definition
|-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
...

Given they are both working with the Symbol File I believe they both belong under the symbol command. I feel like -dump-ast is a little misleading but -dump-clang-ast feels good.

You're right about. I'm sorry, I should have checked what -dump-ast does. It definitely is confusing that it does something completely different than the "dump ast" lldb command.

But what about the second part of my comment? I think it would still be nice if the lldb-test symbols options could be somehow factored into two groups:

• a group which selects *what* to dump: this would be -find, -name, etc.
• a group which selects *how* to dump it: -dump-clang-ast, -dump-ast :/, the default mode

would it be somehow possible to hook in this logic into the findTypes function in lldb-test, so that instead of calling TypeMap::Dump, it does something else if -dump-clang-ast flag is present (this "else" resuling in the printing of clang ast)?

I sympathize with the sentiment that it would be nice to collapse some of the arguments. I spent some time trying to see how to combine these features nicely and I don't think it will end up being any cleaner.

Folding in the -dump-ast and -dump-clang-ast into -find only seems to make sense with -find=type or -find=none so this feels like shoe horning by lumping it with the -find option. Since I now need to add error handling for the blocks, variables and namespaces

If you look at the switch for Find it already has a mess of conflicting options :-( for example -find=block is the only one that use -line.

We won’t be saving much in the way of code just shifting where the if/else will be. Each functionality is really providing different information and I can see how each one is useful, so I don’t want to remove any of them.

I do think the -dump-ast could use a better name maybe -print-decl because it end up using DeclPrinter while -dump-clang-ast end up using ASTDumper.

This utility can be probably be refactored to be cleaner but that is a bigger change and outside the scope of being able to have the ability to verify fixes for how we generate clang ASTs from DWARF which we have no way of doing and I need now to be able to verify a fix I have now.

[labath]

My hope was that something like -find=variable -dump-clang-ast would make sense at least from a theoretical perspective. However, after reading to source code, it looks like that may not be true as (I think) we don't actually create ast nodes for variables. So, making this option a sort of a "top-level" thing may be the best thing to do. Given that now the "symbols" subcommand has a fair set of options, all of which are mutually incompatible (-verify, --dump-ast, -dump-clang-ast, the "default" mode), I think it might make sense to create new subcommands for each of these "modes" ("lldb-test symbols ast") so that they can each get their own options, independently of the others. However, I don't want to hold up other progress over that...

[shafik]

• Simplified DumpFromSymbolFile() based on comments
• Reused -name option in lldb-test and to replace -symbol-name

source/Symbol/ClangASTContext.cpp

9195 ↗

(On Diff #222861)

This is a good point, this also goes for CXXRecordDecl so removed those cases since they are both covered by TagDecl.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

if (auto err = type_system_or_err.takeError()) {
     return {};
   }
 
   DWARFASTParser *dwarf_ast = type_system_or_err->GetDWARFParser();
   if (!dwarf_ast)
     return {};
 
   Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
   TypeSP type_sp = dwarf_ast->ParseTypeFromDWARF(sc, die, log, type_is_new_ptr);
   if (type_sp) {
     GetTypeList().Insert(type_sp);
 
     if (die.Tag() == DW_TAG_subprogram) {
       std::string scope_qualified_name(GetDeclContextForUID(die.GetID())
                                            .GetScopeQualifiedName()
                                            .AsCString(""));
       if (scope_qualified_name.size()) {
         m_function_scope_qualified_name_map[scope_qualified_name].insert(
             die.GetID());
       }
     }
   }
 
   return type_sp;
 }
 
 size_t SymbolFileDWARF::ParseTypes(const SymbolContext &sc,
                                    const DWARFDIE &orig_die,
                                    bool parse_siblings, bool parse_children) {
   size_t types_added = 0;
   DWARFDIE die = orig_die;
+
   while (die) {
+    const dw_tag_t tag = die.Tag();
     bool type_is_new = false;
-    if (ParseType(sc, die, &type_is_new).get()) {
-      if (type_is_new)
-        ++types_added;
-    }
+
+    switch (tag) {
+    case DW_TAG_array_type:
+    case DW_TAG_unspecified_type:
+    case DW_TAG_base_type:
+    case DW_TAG_class_type:
+    case DW_TAG_structure_type:
+    case DW_TAG_union_type:
+    case DW_TAG_enumeration_type:
+    case DW_TAG_subroutine_type:
+    case DW_TAG_subprogram:
+    case DW_TAG_inlined_subroutine:
+    case DW_TAG_pointer_type:
+    case DW_TAG_rvalue_reference_type:
+    case DW_TAG_reference_type:
+    case DW_TAG_typedef:
+    case DW_TAG_ptr_to_member_type:
+      ParseType(sc, die, &type_is_new).get();
+      printf("pubname: %s is_type = %d\n", die.GetPubname(), true);
+      break;
+    default:
+      printf("pubname: %s is_type = %d\n", die.GetPubname(), false);
+      break;
+    }
+
 
     if (parse_children && die.HasChildren()) {
       if (die.Tag() == DW_TAG_subprogram) {
         SymbolContext child_sc(sc);
         child_sc.function = sc.comp_unit->FindFunctionByUID(die.GetID()).get();
         types_added += ParseTypes(child_sc, die.GetFirstChild(), true, true);
       } else
         types_added += ParseTypes(sc, die.GetFirstChild(), true, true);

tools/lldb-test/lldb-test.cpp

 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
+
 #include <cstdio>
 #include <thread>
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace llvm;
 
 namespace opts {
Error opts::symbols::dumpAST(lldb_private::Module &Module) {
   if (!ast_ctx)
     return make_string_error("Can't retrieve AST context.");
 
   auto tu = ast_ctx->getTranslationUnitDecl();
   if (!tu)
     return make_string_error("Can't retrieve translation unit declaration.");
 
   tu->print(outs());
 
+  lldb_private::TypeList type_list;
+  size_t ntypes = symfile->GetTypes(nullptr, eTypeClassAny, type_list);
+  printf("Type list size: %zu\n", ntypes);
+
+  for (size_t i = 0; i < ntypes; ++i) {
+    auto type = type_list.GetTypeAtIndex(i);
+    printf("%s\n", type->GetName().AsCString());
+
+    if (clang::CXXRecordDecl *record_decl = clang_ast_ctx->GetAsCXXRecordDecl(
+            type->GetFullCompilerType().GetOpaqueQualType()))
+      record_decl->dump();
+    else if (clang::TagDecl *tag_decl =
+                 clang_ast_ctx->GetAsTagDecl(type->GetFullCompilerType()))
+      tag_decl->dump();
+    else if (clang::TypedefNameDecl *typedef_decl =
+                 clang_ast_ctx->GetAsTypedefDecl(type->GetFullCompilerType()))
+      typedef_decl->dump();
+    else if (clang::EnumDecl *enum_decl =
+                 clang_ast_ctx->GetAsEnumDecl(type->GetFullCompilerType()))
+      enum_decl->dump();
+    else {
+      clang_ast_ctx
+          ->GetCanonicalQualType(
+              type->GetFullCompilerType().GetOpaqueQualType())
+          .dump();
+    }
+  }
+
   return Error::success();
 }
 
 Error opts::symbols::verify(lldb_private::Module &Module) {
   SymbolFile *symfile = Module.GetSymbolFile();
   if (!symfile)
     return make_string_error("Module has no symbol file.");