[Lldb-commits] [lldb] [lldb/DWARF] Refactor DWARFDIE::Get{Decl, TypeLookup}Context (PR #93291)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/93291 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Refactor DWARFDIE::Get{Decl, TypeLookup}Context (PR #93291)
https://github.com/Michael137 approved this pull request. nice! tests also LGTM https://github.com/llvm/llvm-project/pull/93291 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Refactor DWARFDIE::Get{Decl, TypeLookup}Context (PR #93291)
labath wrote: Good idea. I've added one quick test. I'll add more as I work on these functions further. PTAL https://github.com/llvm/llvm-project/pull/93291 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Refactor DWARFDIE::Get{Decl, TypeLookup}Context (PR #93291)
https://github.com/labath updated
https://github.com/llvm/llvm-project/pull/93291
>From 8463c2433609216499fe5d04c3397a3113071a49 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Fri, 24 May 2024 10:17:40 +
Subject: [PATCH 1/2] [lldb/DWARF] Refactor
DWARFDIE::Get{Decl,TypeLookup}Context
After a bug (the bug is that the functions don't handle DW_AT_signature,
aka type units) led me to one of these similar-but-different functions,
I started to realize that most of the differences between these two
functions are actually bugs.
As a first step towards merging them, this patch rewrites both of them
to follow the same pattern, while preserving all of their differences.
The main change is that GetTypeLookupContext now also uses a `seen` list
to avoid reference loops (currently that's not necessary because the
function strictly follows parent links, but that will change with
DW_AT_signatures).
I've also optimized both functions to avoid recursion by contructing
starting with the deepest scope first (and then reversing it).
---
.../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 197 +-
1 file changed, 104 insertions(+), 93 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 4884374ef9472..03e289bbf3300 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -13,6 +13,7 @@
#include "DWARFDebugInfoEntry.h"
#include "DWARFDeclContext.h"
#include "DWARFUnit.h"
+#include "lldb/Symbol/Type.h"
#include "llvm/ADT/iterator.h"
@@ -379,108 +380,118 @@ std::vector DWARFDIE::GetDeclContextDIEs()
const {
return result;
}
-static std::vector
-GetDeclContextImpl(llvm::SmallSet , DWARFDIE die) {
- std::vector context;
+static void GetDeclContextImpl(DWARFDIE die,
+ llvm::SmallSet ,
+ std::vector ) {
// Stop if we hit a cycle.
- if (!die || !seen.insert(die.GetID()).second)
-return context;
-
- // Handle outline member function DIEs by following the specification.
- if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_specification))
-return GetDeclContextImpl(seen, spec);
-
- // Get the parent context chain.
- context = GetDeclContextImpl(seen, die.GetParent());
+ while (die && seen.insert(die.GetID()).second) {
+// Handle outline member function DIEs by following the specification.
+if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_specification)) {
+ die = spec;
+ continue;
+}
- // Add this DIE's contribution at the end of the chain.
- auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
-context.push_back({kind, ConstString(name)});
- };
- switch (die.Tag()) {
- case DW_TAG_module:
-push_ctx(CompilerContextKind::Module, die.GetName());
-break;
- case DW_TAG_namespace:
-push_ctx(CompilerContextKind::Namespace, die.GetName());
-break;
- case DW_TAG_structure_type:
-push_ctx(CompilerContextKind::Struct, die.GetName());
-break;
- case DW_TAG_union_type:
-push_ctx(CompilerContextKind::Union, die.GetName());
-break;
- case DW_TAG_class_type:
-push_ctx(CompilerContextKind::Class, die.GetName());
-break;
- case DW_TAG_enumeration_type:
-push_ctx(CompilerContextKind::Enum, die.GetName());
-break;
- case DW_TAG_subprogram:
-push_ctx(CompilerContextKind::Function, die.GetName());
-break;
- case DW_TAG_variable:
-push_ctx(CompilerContextKind::Variable, die.GetPubname());
-break;
- case DW_TAG_typedef:
-push_ctx(CompilerContextKind::Typedef, die.GetName());
-break;
- default:
-break;
+// Add this DIE's contribution at the end of the chain.
+auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
+ context.push_back({kind, ConstString(name)});
+};
+switch (die.Tag()) {
+case DW_TAG_module:
+ push_ctx(CompilerContextKind::Module, die.GetName());
+ break;
+case DW_TAG_namespace:
+ push_ctx(CompilerContextKind::Namespace, die.GetName());
+ break;
+case DW_TAG_structure_type:
+ push_ctx(CompilerContextKind::Struct, die.GetName());
+ break;
+case DW_TAG_union_type:
+ push_ctx(CompilerContextKind::Union, die.GetName());
+ break;
+case DW_TAG_class_type:
+ push_ctx(CompilerContextKind::Class, die.GetName());
+ break;
+case DW_TAG_enumeration_type:
+ push_ctx(CompilerContextKind::Enum, die.GetName());
+ break;
+case DW_TAG_subprogram:
+ push_ctx(CompilerContextKind::Function, die.GetName());
+ break;
+case DW_TAG_variable:
+ push_ctx(CompilerContextKind::Variable, die.GetPubname());
+ break;
+case DW_TAG_typedef:
+ push_ctx(CompilerContextKind::Typedef, die.GetName());
+ break;
+default:
+ break;
+}
+// Now process the parent.
+die = die.GetParent();
}
- return context;
}
[Lldb-commits] [lldb] [lldb/DWARF] Refactor DWARFDIE::Get{Decl, TypeLookup}Context (PR #93291)
https://github.com/Michael137 approved this pull request. LGTM! (seems like we could probably add unit-tests for this in `lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp`? But not a blocker) https://github.com/llvm/llvm-project/pull/93291 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/DWARF] Refactor DWARFDIE::Get{Decl, TypeLookup}Context (PR #93291)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
After a bug (the bug is that the functions don't handle DW_AT_signature, aka
type units) led me to one of these similar-but-different functions, I started
to realize that most of the differences between these two functions are
actually bugs.
As a first step towards merging them, this patch rewrites both of them to
follow the same pattern, while preserving all of their differences. The main
change is that GetTypeLookupContext now also uses a `seen` list to avoid
reference loops (currently that's not necessary because the function strictly
follows parent links, but that will change with DW_AT_signatures).
I've also optimized both functions to avoid recursion by contructing starting
with the deepest scope first (and then reversing it).
---
Full diff: https://github.com/llvm/llvm-project/pull/93291.diff
1 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp (+104-93)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 4884374ef9472..03e289bbf3300 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -13,6 +13,7 @@
#include "DWARFDebugInfoEntry.h"
#include "DWARFDeclContext.h"
#include "DWARFUnit.h"
+#include "lldb/Symbol/Type.h"
#include "llvm/ADT/iterator.h"
@@ -379,108 +380,118 @@ std::vector DWARFDIE::GetDeclContextDIEs()
const {
return result;
}
-static std::vector
-GetDeclContextImpl(llvm::SmallSet , DWARFDIE die) {
- std::vector context;
+static void GetDeclContextImpl(DWARFDIE die,
+ llvm::SmallSet ,
+ std::vector ) {
// Stop if we hit a cycle.
- if (!die || !seen.insert(die.GetID()).second)
-return context;
-
- // Handle outline member function DIEs by following the specification.
- if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_specification))
-return GetDeclContextImpl(seen, spec);
-
- // Get the parent context chain.
- context = GetDeclContextImpl(seen, die.GetParent());
+ while (die && seen.insert(die.GetID()).second) {
+// Handle outline member function DIEs by following the specification.
+if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_specification)) {
+ die = spec;
+ continue;
+}
- // Add this DIE's contribution at the end of the chain.
- auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
-context.push_back({kind, ConstString(name)});
- };
- switch (die.Tag()) {
- case DW_TAG_module:
-push_ctx(CompilerContextKind::Module, die.GetName());
-break;
- case DW_TAG_namespace:
-push_ctx(CompilerContextKind::Namespace, die.GetName());
-break;
- case DW_TAG_structure_type:
-push_ctx(CompilerContextKind::Struct, die.GetName());
-break;
- case DW_TAG_union_type:
-push_ctx(CompilerContextKind::Union, die.GetName());
-break;
- case DW_TAG_class_type:
-push_ctx(CompilerContextKind::Class, die.GetName());
-break;
- case DW_TAG_enumeration_type:
-push_ctx(CompilerContextKind::Enum, die.GetName());
-break;
- case DW_TAG_subprogram:
-push_ctx(CompilerContextKind::Function, die.GetName());
-break;
- case DW_TAG_variable:
-push_ctx(CompilerContextKind::Variable, die.GetPubname());
-break;
- case DW_TAG_typedef:
-push_ctx(CompilerContextKind::Typedef, die.GetName());
-break;
- default:
-break;
+// Add this DIE's contribution at the end of the chain.
+auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
+ context.push_back({kind, ConstString(name)});
+};
+switch (die.Tag()) {
+case DW_TAG_module:
+ push_ctx(CompilerContextKind::Module, die.GetName());
+ break;
+case DW_TAG_namespace:
+ push_ctx(CompilerContextKind::Namespace, die.GetName());
+ break;
+case DW_TAG_structure_type:
+ push_ctx(CompilerContextKind::Struct, die.GetName());
+ break;
+case DW_TAG_union_type:
+ push_ctx(CompilerContextKind::Union, die.GetName());
+ break;
+case DW_TAG_class_type:
+ push_ctx(CompilerContextKind::Class, die.GetName());
+ break;
+case DW_TAG_enumeration_type:
+ push_ctx(CompilerContextKind::Enum, die.GetName());
+ break;
+case DW_TAG_subprogram:
+ push_ctx(CompilerContextKind::Function, die.GetName());
+ break;
+case DW_TAG_variable:
+ push_ctx(CompilerContextKind::Variable, die.GetPubname());
+ break;
+case DW_TAG_typedef:
+ push_ctx(CompilerContextKind::Typedef, die.GetName());
+ break;
+default:
+ break;
+}
+// Now process the parent.
+die = die.GetParent();
}
- return context;
}
-std::vector DWARFDIE::GetDeclContext() const {
+std::vector DWARFDIE::GetDeclContext() const {
llvm::SmallSet seen;
- return GetDeclContextImpl(seen,
[Lldb-commits] [lldb] [lldb/DWARF] Refactor DWARFDIE::Get{Decl, TypeLookup}Context (PR #93291)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/93291
After a bug (the bug is that the functions don't handle DW_AT_signature, aka
type units) led me to one of these similar-but-different functions, I started
to realize that most of the differences between these two functions are
actually bugs.
As a first step towards merging them, this patch rewrites both of them to
follow the same pattern, while preserving all of their differences. The main
change is that GetTypeLookupContext now also uses a `seen` list to avoid
reference loops (currently that's not necessary because the function strictly
follows parent links, but that will change with DW_AT_signatures).
I've also optimized both functions to avoid recursion by contructing starting
with the deepest scope first (and then reversing it).
>From 8463c2433609216499fe5d04c3397a3113071a49 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Fri, 24 May 2024 10:17:40 +
Subject: [PATCH] [lldb/DWARF] Refactor DWARFDIE::Get{Decl,TypeLookup}Context
After a bug (the bug is that the functions don't handle DW_AT_signature,
aka type units) led me to one of these similar-but-different functions,
I started to realize that most of the differences between these two
functions are actually bugs.
As a first step towards merging them, this patch rewrites both of them
to follow the same pattern, while preserving all of their differences.
The main change is that GetTypeLookupContext now also uses a `seen` list
to avoid reference loops (currently that's not necessary because the
function strictly follows parent links, but that will change with
DW_AT_signatures).
I've also optimized both functions to avoid recursion by contructing
starting with the deepest scope first (and then reversing it).
---
.../Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 197 +-
1 file changed, 104 insertions(+), 93 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 4884374ef9472..03e289bbf3300 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -13,6 +13,7 @@
#include "DWARFDebugInfoEntry.h"
#include "DWARFDeclContext.h"
#include "DWARFUnit.h"
+#include "lldb/Symbol/Type.h"
#include "llvm/ADT/iterator.h"
@@ -379,108 +380,118 @@ std::vector DWARFDIE::GetDeclContextDIEs()
const {
return result;
}
-static std::vector
-GetDeclContextImpl(llvm::SmallSet , DWARFDIE die) {
- std::vector context;
+static void GetDeclContextImpl(DWARFDIE die,
+ llvm::SmallSet ,
+ std::vector ) {
// Stop if we hit a cycle.
- if (!die || !seen.insert(die.GetID()).second)
-return context;
-
- // Handle outline member function DIEs by following the specification.
- if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_specification))
-return GetDeclContextImpl(seen, spec);
-
- // Get the parent context chain.
- context = GetDeclContextImpl(seen, die.GetParent());
+ while (die && seen.insert(die.GetID()).second) {
+// Handle outline member function DIEs by following the specification.
+if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_specification)) {
+ die = spec;
+ continue;
+}
- // Add this DIE's contribution at the end of the chain.
- auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
-context.push_back({kind, ConstString(name)});
- };
- switch (die.Tag()) {
- case DW_TAG_module:
-push_ctx(CompilerContextKind::Module, die.GetName());
-break;
- case DW_TAG_namespace:
-push_ctx(CompilerContextKind::Namespace, die.GetName());
-break;
- case DW_TAG_structure_type:
-push_ctx(CompilerContextKind::Struct, die.GetName());
-break;
- case DW_TAG_union_type:
-push_ctx(CompilerContextKind::Union, die.GetName());
-break;
- case DW_TAG_class_type:
-push_ctx(CompilerContextKind::Class, die.GetName());
-break;
- case DW_TAG_enumeration_type:
-push_ctx(CompilerContextKind::Enum, die.GetName());
-break;
- case DW_TAG_subprogram:
-push_ctx(CompilerContextKind::Function, die.GetName());
-break;
- case DW_TAG_variable:
-push_ctx(CompilerContextKind::Variable, die.GetPubname());
-break;
- case DW_TAG_typedef:
-push_ctx(CompilerContextKind::Typedef, die.GetName());
-break;
- default:
-break;
+// Add this DIE's contribution at the end of the chain.
+auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
+ context.push_back({kind, ConstString(name)});
+};
+switch (die.Tag()) {
+case DW_TAG_module:
+ push_ctx(CompilerContextKind::Module, die.GetName());
+ break;
+case DW_TAG_namespace:
+ push_ctx(CompilerContextKind::Namespace, die.GetName());
+ break;
+case DW_TAG_structure_type:
+ push_ctx(CompilerContextKind::Struct, die.GetName());
+ break;
+
