[Lldb-commits] [PATCH] D126668: LLDB: Fix resolving nested template parameters

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126668

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126668: LLDB: Fix resolving nested template parameters

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

> 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 
in DWARF) and empty parameter packs (which is the edge case the original patch 
fixed).

Example: this is currently rejected even though we have all the required 
information:

  template
  struct X {};
  
  int main() {
X x;
  }

->

  0x0059:   DW_TAG_structure_type
  DW_AT_calling_convention(DW_CC_pass_by_value)
  DW_AT_name  ("X")
  DW_AT_byte_size (0x01)
  DW_AT_decl_file 
("/home/teemperor/test/someoneshouldreallypaymeforthis.cpp")
  DW_AT_decl_line (2)
  
  0x0062: DW_TAG_GNU_template_parameter_pack
DW_AT_name("T")
  
  0x0067:   DW_TAG_template_type_parameter
  DW_AT_type  (0x0052 "int")
  
  0x006c:   NULL
  
  0x006d: NULL
  
  0x006e:   NULL

I think having a fake class with a weird identifiers that contains template 
arguments is better than the missing arguments, so this looks in general good 
to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126668

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126668: LLDB: Fix resolving nested template parameters

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126668

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D126668: LLDB: Fix resolving nested template parameters

[Levon Ter-Grigoryan via Phabricator via lldb-commits]

PatriosTheGreat created this revision.
PatriosTheGreat added a reviewer: teemperor.
Herald added a reviewer: shafik.
Herald added a project: All.
PatriosTheGreat requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Right now LLDB ignores nested template type:

echo "
template 
struct A {};
int main() {

  A> 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**>) s = {}

This issue was introduced in LLDB 12 due to this CL: 
https://reviews.llvm.org/D92425
Before LLDB 12 this type was resolving correctly:
(A** >) s = {}

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

Apparently in this case Clang emits A as a forward declaration:
0x05b4:   DW_TAG_base_type

  DW_AT_name  ("int")
  DW_AT_encoding  (DW_ATE_signed)
  DW_AT_byte_size (0x04)

0x05bb:   DW_TAG_structure_type

  DW_AT_calling_convention(DW_CC_pass_by_value)
  DW_AT_name  ("A >")
  DW_AT_byte_size (0x01)
  DW_AT_decl_file ("/home/teemperor/test/args.cpp")
  DW_AT_decl_line (2)

0x05c4: DW_TAG_template_type_parameter

  DW_AT_type(0x05ce "A")
  DW_AT_name("T")

0x05cd: NULL

0x05ce:   DW_TAG_structure_type

  DW_AT_name  ("A")
  DW_AT_declaration   (true)

0x05d3:   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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126668

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py


Index: lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
===
--- lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
+++ lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -47,7 +47,7 @@
 # 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 *", 
"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 *")
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2050,6 +2050,8 @@
   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();
 }
 


Index: lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
===
--- lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
+++ lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -47,7 +47,7 @@
 # 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 *", "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 *")
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2050,6 +2050,8 @@
   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();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits