[Lldb-commits] [PATCH] D156774: [WIP][lldb] Parse enums while parsing a type

[Vlad Serebrennikov via Phabricator via lldb-commits]

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

Fixes #64291 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156774

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


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3037,6 +3037,17 @@
module_sp, base_classes, layout_info);
   break;
 
+case DW_TAG_enumeration_type:
+{
+  SymbolContextScope *scope;
+  scope = parent_die.GetDWARF()->GetObjectFile()->GetModule().get();
+  assert(scope);
+  SymbolContext sc(scope);
+  ParsedDWARFTypeAttributes attrs{die};
+  ParseEnum(sc, die, attrs);
+}
+break;
+
 default:
   break;
 }


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3037,6 +3037,17 @@
module_sp, base_classes, layout_info);
   break;
 
+case DW_TAG_enumeration_type:
+{
+  SymbolContextScope *scope;
+  scope = parent_die.GetDWARF()->GetObjectFile()->GetModule().get();
+  assert(scope);
+  SymbolContext sc(scope);
+  ParsedDWARFTypeAttributes attrs{die};
+  ParseEnum(sc, die, attrs);
+}
+break;
+
 default:
   break;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D156774: [WIP][lldb] Parse enums while parsing a type

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3043
+  SymbolContextScope *scope;
+  scope = parent_die.GetDWARF()->GetObjectFile()->GetModule().get();
+  assert(scope);

I've used 
https://github.com/llvm/llvm-project/blob/fa2b038cadf17d08014e5fb75c47b5024860953e/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L2689
 as a reference, and I guess I'm missing CompileUnit counterpart of this. 
Surprisingly to me, even with CompileUnit part missing, I was able to debug a 
Clang crash successfully.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [WIP][lldb] Parse enums while parsing a type

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill added a comment.

This patch is intentionally aimed squarely at enums.
But I'd like lldb to address the issue of accessing compile-time constants 
holistically. Or at least agree on a path forward for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb] Parse enums while parsing a type

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3040
 
+case DW_TAG_enumeration_type:
+{

Michael137 wrote:
> Michael137 wrote:
> > Michael137 wrote:
> > > Michael137 wrote:
> > > > At first glance this seems OK but I'll want to check why the type 
> > > > doesn't get resolved when `clang::Sema` asks LLDB for it by name
> > > Checking locally and will report back
> > Ok, so what happens for `expr Outer::Enum::var` is that LLDB will first 
> > resolve the `Outer` structure (and all its member fields, but not nested 
> > types). When clang looks for `Enum`, it wants to find it in a direct lookup 
> > into the `Outer` DeclContext. But that lookup will fail because we never 
> > created the decls for the nested type. There is no fallback to the external 
> > source in such a case unfortunately so LLDB never has the chance to correct 
> > this. See the `LookupQualifiedName` code path in 
> > `Sema::BuildCXXNestedNameSpecifier`.
> > 
> > So I think this proposed approach should be fine. But what I think could be 
> > better is if we just do the same for `DW_TAG_enumeration_type` and 
> > `DW_TAG_structure_type` as we do for `DW_TAG_subprogram`.
> > 
> > I.e., simply push back the type into `member_function_dies` (we should 
> > rename this) and then it will get resolved properly in `CompleteRecordType`.
> Also `DW_TAG_union_type` while you're here :)
I can look into doing things more lazily like `DW_TAG_subprogram`, if you can 
point me to code paths for this on LLDB side:
> When clang looks for Enum, it wants to find it in a direct lookup into the 
> Outer DeclContext. But that lookup will fail because we never created the 
> decls for the nested type. There is no fallback to the external source in 
> such a case unfortunately so LLDB never has the chance to correct this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill added a comment.

In D156774#4572947 , @Michael137 
wrote:

> Are you still planning on moving this forward? Otherwise I could commandeer 
> the revision to get this in. I do think it's a useful bug to address

I do. Locally I've been preparing additional changes on top of this patch that 
expose enums in SBType, so that I can do end-to-end test to ensure this 
sufficiently addresses my use case.
On top of that, I'm planning to redo this patch after the example of 
`DW_TAG_subprogram` per your suggestion.

Unfortunately, I didn't have time last week for this, and now I'm knee deep 
into triaging old Clang bugs. I'm not finished with that until number of open 
issues is brought under 20k :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill added a comment.

I tested this patch together with the following new code:

  uint32_t TypeSystemClang::GetNumMemberEnums(lldb::opaque_compiler_type_t 
type) {
using EnumIt = clang::DeclContext::specific_decl_iterator;
if (!type)
  return 0;
clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
clang::DeclContext *ctx = qual_type->getAsRecordDecl()->getDeclContext();
return std::distance(EnumIt(ctx->decls_begin()), EnumIt(ctx->decls_end()));
  }
  
  CompilerType
  TypeSystemClang::GetMemberEnumAtIndex(lldb::opaque_compiler_type_t type,
size_t index) {
using EnumIt = clang::DeclContext::specific_decl_iterator;
if (!type)
  return CompilerType();
  
clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
clang::DeclContext *ctx = qual_type->getAsRecordDecl()->getDeclContext();
size_t enum_index = 0;
for (EnumIt enums_it(ctx->decls_begin()), enums_end(ctx->decls_end());
 enums_it != enums_end;
 ++enums_it, ++enum_index) {
if (enum_index == index) {
  return CompilerType(weak_from_this(), *enums_it);
}
}
  }

I created all the wrappers to make it available in Python. The result was 
unsatisfactory: this code doesn't even trigger 
`DWARFASTParserClang::ParseChildMembers` that this patch touches, and return 0 
instead of 1. This doesn't change even if I trigger `ParseChildMembers` via 
other means before asking for a number of member enums in a type.

Code I tested this on:

  using intptr_t = long;
  using uintptr_t = unsigned long;
  
  struct PointerIntPairInfo {
enum MaskAndShiftConstants : uintptr_t {
  PointerBitMask =
  ~(uintptr_t)(((intptr_t)1 << 3) - 1),
};
  
int a{};
  };
  
  static uintptr_t dummy() {
return PointerIntPairInfo::PointerBitMask;
  }
  
  int main()
  {
  PointerIntPairInfo p;
  __builtin_debugtrap();
  return p.a + foo();
  }

If you have any suggestions what I missed or did wrong, please let me know.

I'll continue with this patch nevertheless, but it's clear now that there's 
still a way to go until I can access that enum without going through slow 
expression evaluator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill updated this revision to Diff 551811.
Endill added a comment.

Follow the `DW_TAG_subprogram` approach for `DW_TAG_enum_type`, 
`DW_TAG_structure_type`, and `DW_TAG_union_type`: rely on `ParseType()` called 
afterwards instead of eagerly parsing them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -157,7 +157,7 @@
   bool ParseChildMembers(
   const DWARFDIE &die, lldb_private::CompilerType &class_compiler_type,
   std::vector> &base_classes,
-  std::vector &member_function_dies,
+  std::vector &member_function_and_type_dies,
   DelayedPropertyList &delayed_properties,
   const lldb::AccessType default_accessibility,
   lldb_private::ClangASTImporter::LayoutInfo &layout_info);
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2149,14 +2149,14 @@
 
 std::vector> bases;
 // Parse members and base classes first
-std::vector member_function_dies;
+std::vector member_function_and_type_dies;
 
 DelayedPropertyList delayed_properties;
-ParseChildMembers(die, clang_type, bases, member_function_dies,
+ParseChildMembers(die, clang_type, bases, member_function_and_type_dies,
   delayed_properties, default_accessibility, layout_info);
 
-// Now parse any methods if there were any...
-for (const DWARFDIE &die : member_function_dies)
+// Now parse any methods or nested types if there were any...
+for (const DWARFDIE &die : member_function_and_type_dies)
   dwarf->ResolveType(die);
 
 if (type_is_objc_object_or_interface) {
@@ -2999,7 +2999,7 @@
 bool DWARFASTParserClang::ParseChildMembers(
 const DWARFDIE &parent_die, CompilerType &class_clang_type,
 std::vector> &base_classes,
-std::vector &member_function_dies,
+std::vector &member_function_and_type_dies,
 DelayedPropertyList &delayed_properties,
 const AccessType default_accessibility,
 ClangASTImporter::LayoutInfo &layout_info) {
@@ -3028,8 +3028,11 @@
   break;
 
 case DW_TAG_subprogram:
+case DW_TAG_enumeration_type:
+case DW_TAG_structure_type:
+case DW_TAG_union_type:
   // Let the type parsing code handle this one for us.
-  member_function_dies.push_back(die);
+  member_function_and_type_dies.push_back(die);
   break;
 
 case DW_TAG_inheritance:


Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -157,7 +157,7 @@
   bool ParseChildMembers(
   const DWARFDIE &die, lldb_private::CompilerType &class_compiler_type,
   std::vector> &base_classes,
-  std::vector &member_function_dies,
+  std::vector &member_function_and_type_dies,
   DelayedPropertyList &delayed_properties,
   const lldb::AccessType default_accessibility,
   lldb_private::ClangASTImporter::LayoutInfo &layout_info);
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2149,14 +2149,14 @@
 
 std::vector> bases;
 // Parse members and base classes first
-std::vector member_function_dies;
+std::vector member_function_and_type_dies;
 
 DelayedPropertyList delayed_properties;
-ParseChildMembers(die, clang_type, bases, member_function_dies,
+ParseChildMembers(die, clang_type, bases, member_function_and_type_dies,
   delayed_properties, default_accessibility, layout_info);
 
-// Now parse any methods if there were any...
-for (const DWARFDIE &die : member_function_dies)
+// Now parse any methods or nested types if there were any...
+for (const DWARFDIE &die : member_function_and_type_dies)
   dwarf->ResolveType(die);
 
 if (type_is_objc_object_or_interface) {
@@ -2999,7 +2999,7 @@
 bool DWARFASTParserClang::ParseChildMembers(
 const DWARFDIE &parent_die, CompilerType &class_clang_type,
 std::vector> &base_classes,
-std::vector &member_function_dies,
+std::vector &member_function_and_type_dies,
 Delay

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill added a comment.

In D156774#4601736 , @Michael137 
wrote:

> What were your lldb commands when you tested this?

`script import lldb; frame = lldb.thread.GetFrameAtIndex(0); 
print(frame.variables[0].type.GetNumberOfMemberEnums())`

> LLDB currently completes types lazily when it thinks it can. Does your new 
> API still fail if you run `expr p` prior? (the idea is that that would 
> trigger completion of the type and parse dwarf). If we dont ever call 
> `GetFullCompilerType` on your type LLDB will never try to pull in the 
> definition

No amount of tinkering with `expr` makes `script 
print(frame.variables[0].type.GetNumberOfMemberEnums())` output a non-zero 
value.
I tested this with the changes I uploaded here half an hour ago.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill added a comment.

In D156774#4601830 , @Michael137 
wrote:

> I think you may want to use `GetCompleteQualType` before iterating the 
> DeclContext; that's how some of the other TypeSystemClang APIs do it. That 
> will make sure we complete the type by the time we look for the enums.

I tried to incorporate this function into my version of 
`GetNumberOfMemberEnums`, but it didn't work out.

> E.g.,:
>
>   clang::QualType qual_type = 
> RemoveWrappingTypes(GetCanonicalQualType(type));  
>  
>   if (GetCompleteQualType(&getASTContext(), qual_type)) { 
> 
> const clang::RecordType *record_type =
>  
> llvm::cast(qual_type.getTypePtr());
> 
> const clang::RecordDecl *record_decl = record_type->getDecl();
>  
> assert(record_decl);  
>  
> return std::distance(EnumIt(record_decl->decls_begin()), 
> EnumIt(record_decl->decls_end())); 
>   }   
>  
>
> Will review it more thoroughly on tomorrow though

But your version actually works, returning 1! Thank you very much!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill added a comment.

Ping @Michael137


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill added a comment.

In D156774#4644623 , @Michael137 
wrote:

> Also, I assume the extra changes to make the PointerIntPair formatter work 
> will be in a follow-up patch?

Yes. That work is not finished yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill added a comment.

@Michael137 I moved this over to GitHub 
, and I need your help there 
writing the unit test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D156774: [lldb][DWARFASTParserClang] Resolve nested types when parsing structures

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill abandoned this revision.
Endill added a comment.

Moved to GitHub: https://github.com/llvm/llvm-project/pull/66879


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156774

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

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

[Vlad Serebrennikov via Phabricator via lldb-commits]

Endill added a comment.

In D140996#4189452 , @bolshakov-a 
wrote:

> Sorry! It's my first time using Phabricator. Maybe, the problem occurs 
> because I've solved the issue with Arcanist just by means of copy-pasting 
> patches into "Update Diff" Web GUI form.

There's nothing wrong with copy-pasting into web GUI form. You just have to 
export the patch with the right options, e.g. `git diff HEAD~1 -U99 > 
mypatch.patch` (more on this here 
).

> Maybe, I should reopen the PR?

You can simply update the review with properly exported patch.


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

https://reviews.llvm.org/D140996

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