aeubanks added inline comments.
================
Comment at:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1558-1560
+ if (template_param_infos.hasParameterPack()) {
+ args = template_param_infos.packed_args->args;
+ }
----------------
dblaikie wrote:
> I think there's some existing problems with lldb here (& gdb, as it happens)
> - this doesn't specify where the parameter pack is in the argument list
> (looks like it assumes it's the only template parameters?) - and doesn't
> handle the possibility of multiple parameter packs in a single parameter
> list, which I think is possible in some corner cases.
>
> I think maybe the existing lldb code can handle some non-pack parameters
> followed by a single pack, but this code assumes it's either non-pack or a
> pack, never both - so might be more restrictive than the existing
> limitations? But maybe not - I might've misread/misremembered the other lldb
> code.
you're right, fixed to check both, and added test case
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1585
+ DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
+ // TODO: change this to get the correct decl context parent....
+ while (parent_decl_ctx_die) {
----------------
Michael137 wrote:
> dblaikie wrote:
> > What's incorrect about it at the moment?
> >
> > Oh, I see this code has just moved around.
> Why move this into `ParseStructureLikeDIE`? Just to remove the need for a
> mutable `std::string&` param? I guess access to `die`? Might be easier to
> review (and maintain) as a helper function still
so that we have access to `ParseTemplateParameterInfos`, which calls
`ParseTemplateDIE` which creates clang types and stuffs them into
`TemplateParameterInfos`, which we use to create the template parameters string.
moved into helper function
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2498-2501
+ llvm::StringRef qualNameTemplateParams =
+ qualName.slice(it, qualName.size());
+ if (templateParams != qualNameTemplateParams)
+ return true;
----------------
dblaikie wrote:
> And this checks the stringified template params match exactly - so there's
> opportunity for improvement in the future to compare with more fuzzy matching
> (I guess we can't use clang's AST because that'd involve making two instances
> of the type somehow which doesn't make a lot of sense) so that users don't
> have to spell the template spec exactly the same way clang does (eg: `x<int,
> int>` versus `x<int,int>` - or other more complicated situations with
> function pointers, parentheses, casts, etc.
lldb already requires exact name matching when looking up classes
e.g.
```
$ cat /tmp/a.cc
template<class T>struct Foo {};
int main() {
Foo<int> a;
Foo<long> b;
Foo<float> c;
}
template<class T>struct Foo {};
int main() {
Foo<int> a;
Foo<long> b;
Foo<float> c;
}
~/repos/llvm-project/build/cmake$ ./bin/lldb /tmp/a
(lldb) target create "/tmp/a"
Current executable set to '/tmp/a' (x86_64).
(lldb) im loo -t 'Foo< int>'
(lldb) im loo -t 'Foo<int>'
1 match found in /tmp/a:
id = {0x00000058}, name = "Foo<int>", byte-size = 1, decl = a.cc:1,
compiler_type = "template<> struct Foo<int> {
}"
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134378/new/
https://reviews.llvm.org/D134378
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
