[Lldb-commits] [lldb] [lldb] Make CompilerDecl::GetName (always) return template args (PR #116068)

[Pavel Labath via lldb-commits]

labath wrote:

> Very interesting find, I _think_ this makes sense since the `FindTypes` query expects to find template parameters if we're looking up templates. And given this API is used specifically to perform these kinds of queries, this LGTM. But please confirm if my understanding is correct:
> 
> In the non-simple names case, when we stop at the breakpoint and start formatting the function arguments, LLDB creates a `CXXRecordDecl` with the name `bar<int>` in the shared library TypeSystem because it only sees a `DW_TAG_structure_type "bar<int>"` forward declaration without any template parameter children DIEs. Then when we call `FindCompleteType`, the `Decl`s `DeclarationName` would've been set to `bar<int>`, so the `TypeQuery` will be able to match the `bar<int>` definition using the index. When `FindTypes` resolves the definition DIE we get from the index, it triggers the creation of a `ClassTemplateSpecializationDecl` in the main module's TypeSystem and everything works out.
> 
> But in the simple names case we create a `ClassTemplateSpecializationDecl` in the shared library module (as opposed to a plain `CXXRecordDecl`), because the forward declaration with `-gsimple-template-names` has a template parameter DIE. But that declaration name is `bar`, because that's what DWARF told us the `DeclarationName` of this `ClassTemplateSpecializationDecl` was. But then `FindTypes` doesn't know what to do. It finds the `bar` definition DIE from the index, but we actually end up _not_ doing a `simple-template-names` lookup because [UpdateCompilerContextForSimpleTemplateNames](https://github.com/llvm/llvm-project/blob/593be023615a456ca6ee0ef9bedc21301d73b73c/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L2816) is only true if the query contained template parameters.

That is all correct. 

> 
> So I guess my question is, why is a `TypeQuery` by basename without template parameters not supported if we compiled with `-gsimple-template-names`? Despite us being able to find the definition DIE in the index? I guess the idea of `UpdateCompilerContextForSimpleTemplateNames` was to avoid doing a double-lookup if the first `FindTypes` failed for a template basename? (I vaguely remember this coming up on the original review)

I think the problem here is of a more fundamental nature. `FindTypes` finds **types**. `bar` is *not* a type. `bar<int>` is. So, while a type query for `bar` will find the definition DIE with `DW_AT_name="bar"`, that DIE will actually be defining the type `bar<int>`. So when the implementation looks at the type constructed from that DIE, it will see that the name does not match what is being asked (it gets this name through a different API, so it will include the template args even without this patch), and discard the type.

As far as i know, this is all WAI. Existing FindType callers expect to get a specific type as a result of their query -- not a collection of instantiations of that template. (That's definitely true in this case, where returning the collection of instantiations would just push the burden of filtering them onto the caller.) That's not to say this kind of a template search is not useful. Having something like that would go a long way towards making expressions like `(bar<int> *) ptr` work. But that would probably be a different API and the problem is that this is basically impossible to implement in a non-simplified-template-name world, and even with simplified names, returning all instantiations might be too expensive.

https://github.com/llvm/llvm-project/pull/116068