LLDB: Fix resolving nested template parameters

[PatriosTheGreat]

Summary

Right now LLDB ignores nested template type:

echo "
template <typename T>
struct A {};
int main() {
  A<A<int>> s;
}
" > sample.cc

clang++ sample.cc -g -O0
lldb-15 a.out -o "breakpoint set -l 6 -f sample.cc" -o "run" -o "frame variable"

The result:
(A<A<>>) s = {}

It looks like this CL introduced this behavior: https://reviews.llvm.org/D92425
Before the LLDB was resolving this type correctly:

lldb-11 a.out -o "breakpoint set -l 6 -f sample.cc" -o "run" -o "frame variable"
(A<A<int>) s = {}

I discussed this issue with Raphael in discord:
https://discord.com/channels/636084430946959380/636732809708306432/980825811714191371

Apparently in this case Clang emits A<int> as a forward declaration:

./llvm-dwarfdump a.out

0x000005b4:   DW_TAG_base_type
                DW_AT_name      ("int")
                DW_AT_encoding  (DW_ATE_signed)
                DW_AT_byte_size (0x04)
0x000005bb:   DW_TAG_structure_type
                DW_AT_calling_convention        (DW_CC_pass_by_value)
                DW_AT_name      ("A<A<int> >")
                DW_AT_byte_size (0x01)
                DW_AT_decl_file ("/home/teemperor/test/args.cpp")
                DW_AT_decl_line (2)

0x000005c4:     DW_TAG_template_type_parameter
                  DW_AT_type    (0x000005ce "A<int>")
                  DW_AT_name    ("T")

0x000005cd:     NULL

0x000005ce:   DW_TAG_structure_type
                DW_AT_name      ("A<int>")
                DW_AT_declaration       (true)

0x000005d3:   NULL

So for LLDB it looks the same as template with empty arguments.
Turning back the old logic is fixing this issue. Other tests from LLVM test suite on Linux seems to be green.

[labath]

I don't really have the full context here, but I am wondering if we shouldn't somehow take the DW_AT_declaration attribute into account here. It seems like that should give a more definitive answer as to whether we can expect to see a full set of template parameters or not.

[teemperor]

I don't really have the full context here, but I am wondering if we shouldn't somehow take the DW_AT_declaration attribute into account here. It seems like that should give a more definitive answer as to whether we can expect to see a full set of template parameters or not.

I think that is actually a good approach. I think the current patch rejects forward declarations (which is fine as we don't have the required information to make a template) and empty parameter packs (which is bad as this currently works).

Example: This is currently rejected with this patch (-> we end up creating a class named X<int>) even though we have all the required information:

template<typename ...T>
struct X {};

int main() {
  X<int> x;
}

->

0x00000059:   DW_TAG_structure_type
                DW_AT_calling_convention        (DW_CC_pass_by_value)
                DW_AT_name      ("X<int>")
                DW_AT_byte_size (0x01)
                DW_AT_decl_file ("/home/teemperor/test/someoneshouldreallypaymeforthis.cpp")
                DW_AT_decl_line (2)

0x00000062:     DW_TAG_GNU_template_parameter_pack
                  DW_AT_name    ("T")

0x00000067:       DW_TAG_template_type_parameter
                    DW_AT_type  (0x00000052 "int")

0x0000006c:       NULL

0x0000006d:     NULL

0x0000006e:   NULL

So I would say we just bail out if we're asked to parse any template declaration and just do the fallback approach that creates a fake class.

[dblaikie]

FWIW, clang has support for including template parameter DIEs for template declarations (-Xclang -debug-forward-template-params). We could enable that by default when tuning for lldb (or maybe we're at the tipping point and we should enable it by default in general - I pushed back on that originally when Sony folks added/proposed it, under the argument that other debuggers didn't need this info - but if they do need the info, so be it)

That feature is also turned on by default in -gsimple-template-names which we're currently working on adding support to lldb for - so ensuring that lldb does the best things when declarations do have template parameters would be good. So not filtering based on declaration, but I guess based on "<>" in the name and the presenec/absence of template parameter DIEs as the patch does currently?

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

for (DWARFDIE die : parent_die.children()) {
     case DW_TAG_GNU_template_template_param:
       ParseTemplateDIE(die, template_param_infos);
       break;
 
     default:
       break;
     }
   }
+  if (template_param_infos.args.empty() && template_param_infos.names.empty())
+    return false;
   return template_param_infos.args.size() == template_param_infos.names.size();
 }
 
 bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
                                              lldb_private::Type *type,
                                              CompilerType &clang_type) {
   const dw_tag_t tag = die.Tag();
   SymbolFileDWARF *dwarf = die.GetDWARF();

lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py

def test_forward_declarations(self):
         self.assertCompleteWithVar("EmptyClass")
         self.assertCompleteWithVar("DefinedClass")
         self.assertCompleteWithVar("DefinedClassTypedef")
         self.assertCompleteWithVar("DefinedTemplateClass<int>")
 
         # Record types without a defining declaration are not complete.
         self.assertPointeeIncomplete("FwdClass *", "fwd_class")
         self.assertPointeeIncomplete("FwdClassTypedef *", "fwd_class_typedef")
-        self.assertPointeeIncomplete("FwdTemplateClass<> *", "fwd_template_class")
+        self.assertPointeeIncomplete("FwdTemplateClass<int> *", "fwd_template_class")
 
         # A pointer type is complete even when it points to an incomplete type.
         fwd_class_ptr = self.expect_expr("fwd_class", result_type="FwdClass *")
         self.assertTrue(fwd_class_ptr.GetType().IsTypeComplete())
 
     @no_debug_info_test
     def test_builtin_types(self):
         """ Tests builtin types and types derived from them. """