[Lldb-commits] [PATCH] D56462: Change SymbolFile::ParseTypes to ParseTypesForCompileUnit

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D56462#1351394 <https://reviews.llvm.org/D56462#1351394>, @clayborg wrote:

> But if everyone else thinks differently, I'll go with the majority. Just my initial thoughts.


IIRC, the original point of this ParseTypes function was to have a way for lldb developers to force lldb to parse all the types in some interesting context.  The equivalent facility had proved useful in gdb (maint print symbols) to trigger obscure type parsing issues directly.  We didn't add a command for it since most often you are debugging lldb with lldb so you can just call it by hand.

When ParseTypes(SymbolContext &sc) gets used in ParseAllDebugSymbols, the calling code was already doing the iteration over CU's to dump all the types in the module.  The code to do that iteration is quite simple, so I don't see the need to fold it into the ParseTypes function, and when you want to do the CU iteration by hand while debugging, you can call ParseAllDebugSymbols.

I'm not sure how useful "parsing all the types in a function" is, as types aren't usually defined in functions and so forcing just the ones in the function to get realized NOW isn't all that helpful.  This might be more useful if it parsed all the types referred to (i.e. any DW_AT_type), but I don't think that's what it does.  It just handles the various DW_TAG_some_kind_of_type DIEs.

So I agree with Zachary that supporting all the possibilities in a SymbolContext seems overly complex to define and not all that helpful.  ParseTypesForCompileUnit seems like the right level to do this.  It might be nice to have a "Module::ParseAllDebugTypes" for calling by hand, since "Module::ParseAllDebugSymbols" does way more work than needed if you know you are just interested in type parsing.  But that's a minor thing.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56462