[Lldb-commits] [PATCH] D133446: [LLDB][NativePDB] Global ctor and dtor should be global decls.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D133446#3787362 <https://reviews.llvm.org/D133446#3787362>, @zequanwu wrote:

> In D133446#3786561 <https://reviews.llvm.org/D133446#3786561>, @labath wrote:
>
>> In D133446#3780961 <https://reviews.llvm.org/D133446#3780961>, @zequanwu wrote:
>>
>>> In D133446#3779600 <https://reviews.llvm.org/D133446#3779600>, @labath wrote:
>>>
>>>> I believe that this fixes the crash, but the names of generated functions still look fairly weird. Could we create them using their mangled name instead? That way, someone might actually call them, if he was so inclined.
>>>
>>> It looks like they don't have mangled name stored in pdb.
>>
>> Hmm.. That's unfortunate. In that case, I'm wondering if this shouldn't be handled somehow directly inside `MSVCUndecoratedNameParser`. What does the function return right now? Do you think that result is going to be useful for other users of that class?
>
> `MSVCUndecoratedNameParser` expects a undercoated name of a entity(class, function, variable, etc) and returns the a pair of {pointer to parent decl context, base name of the entity}.

Hmm... are you sure about that? `CreateDeclInfoForUndecoratedName` returns a {decl ctx, base name} pair. `MSVCUndecoratedNameParser` returns (creates) an array of `MSVCUndecoratedNameSpecifier`s. My question was about what those specifiers are specifically for the inputs we're talking about here (`static void B::dynamic atexit destructor for 'glob'()` or such), and whether those outputs "make sense" for some user of that class (it's clear that they don't make sense here).

Basically, I'm trying to see whether it's possible (desirable?) to move this logic into the parser class. The parser already does a lot of string manipulation, so checking for these strings there would make more sense to me than doing some sort of a pre-parse from the outside.

>> Also, I'm still continuing to be amazed (and scared) by the amount of name parsing that is going on here. Have you checked that there's no better way to associate an entity with its enclosing context?
>
> If the entity is a global variable, we don't have any other ways to know if its enclosing context is record or namespace.  If the entity is record or function, then we can know if its enclosing context is a record by looking at its type info but need to fall back to parse name again if it's inside a namespace. The issue is there isn't a way to represent namespaces in PDB, and they are just embedded into the name strings.

Hmm.. well, that's unfortunate, but I guess we'll have to live with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133446