[Lldb-commits] [PATCH] D73787: [NFC] Refactor `GetDWARFDeclContext` to return `DWARFDeclContext`

[Jan Kratochvil via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d11d5f62422: [lldb] [NFC] Refactor GetDWARFDeclContext to 
return DWARFDeclContext (authored by jankratochvil).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73787

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

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -308,8 +308,7 @@
   static lldb_private::CompilerDeclContext
   GetContainingDeclContext(const DWARFDIE );
 
-  static void GetDWARFDeclContext(const DWARFDIE ,
-  DWARFDeclContext _decl_ctx);
+  static DWARFDeclContext GetDWARFDeclContext(const DWARFDIE );
 
   static lldb::LanguageType LanguageTypeFromDWARF(uint64_t val);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2959,8 +2959,8 @@
 }
 
 if (try_resolving_type) {
-  DWARFDeclContext type_dwarf_decl_ctx;
-  GetDWARFDeclContext(type_die, type_dwarf_decl_ctx);
+  DWARFDeclContext type_dwarf_decl_ctx =
+  GetDWARFDeclContext(type_die);
 
   if (log) {
 GetObjectFile()->GetModule()->LogMessage(
@@ -3377,12 +3377,10 @@
 // declaration context.
 if ((parent_tag == DW_TAG_compile_unit ||
  parent_tag == DW_TAG_partial_unit) &&
-Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU( {
-  DWARFDeclContext decl_ctx;
-
-  GetDWARFDeclContext(die, decl_ctx);
-  mangled = decl_ctx.GetQualifiedNameAsConstString().GetCString();
-}
+Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(
+  mangled = GetDWARFDeclContext(die)
+.GetQualifiedNameAsConstString()
+.GetCString();
   }
 
   if (tag == DW_TAG_formal_parameter)
@@ -3984,14 +3982,13 @@
   return CompilerDeclContext();
 }
 
-void SymbolFileDWARF::GetDWARFDeclContext(const DWARFDIE ,
-  DWARFDeclContext _decl_ctx) {
-  if (!die.IsValid()) {
-dwarf_decl_ctx.Clear();
-return;
-  }
+DWARFDeclContext SymbolFileDWARF::GetDWARFDeclContext(const DWARFDIE ) {
+  if (!die.IsValid())
+return {};
+  DWARFDeclContext dwarf_decl_ctx =
+  die.GetDIE()->GetDWARFDeclContext(die.GetCU());
   dwarf_decl_ctx.SetLanguage(GetLanguage(*die.GetCU()));
-  die.GetDIE()->GetDWARFDeclContext(die.GetCU(), dwarf_decl_ctx);
+  return dwarf_decl_ctx;
 }
 
 LanguageType SymbolFileDWARF::LanguageTypeFromDWARF(uint64_t val) {
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -158,8 +158,7 @@
 return HasChildren() ? this + 1 : nullptr;
   }
 
-  void GetDWARFDeclContext(DWARFUnit *cu,
-   DWARFDeclContext _decl_ctx) const;
+  DWARFDeclContext GetDWARFDeclContext(DWARFUnit *cu) const;
 
   DWARFDIE GetParentDeclContextDIE(DWARFUnit *cu) const;
   DWARFDIE GetParentDeclContextDIE(DWARFUnit *cu,
@@ -169,6 +168,9 @@
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
 protected:
+  static DWARFDeclContext
+  GetDWARFDeclContextStatic(const DWARFDebugInfoEntry *die, DWARFUnit *cu);
+
   dw_offset_t m_offset; // Offset within the .debug_info/.debug_types
   uint32_t m_parent_idx; // How many to subtract from "this" to get the parent.
  // If zero this die has no parent
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -868,21 +868,30 @@
   }
 }
 
-void DWARFDebugInfoEntry::GetDWARFDeclContext(
-DWARFUnit *cu, DWARFDeclContext _decl_ctx) const {
-  const dw_tag_t tag = Tag();
-  if (tag == DW_TAG_compile_unit || tag == DW_TAG_partial_unit)
-return;
-  dwarf_decl_ctx.AppendDeclContext(tag, GetName(cu));
-  DWARFDIE parent_decl_ctx_die = GetParentDeclContextDIE(cu);
-  if (parent_decl_ctx_die && parent_decl_ctx_die.GetDIE() 

[Lldb-commits] [PATCH] D73787: [NFC] Refactor `GetDWARFDeclContext` to return `DWARFDeclContext`

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This looks good too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73787



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

[Lldb-commits] [PATCH] D73787: [NFC] Refactor `GetDWARFDeclContext` to return `DWARFDeclContext`

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 242936.
This revision is now accepted and ready to land.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73787

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

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -308,8 +308,7 @@
   static lldb_private::CompilerDeclContext
   GetContainingDeclContext(const DWARFDIE );
 
-  static void GetDWARFDeclContext(const DWARFDIE ,
-  DWARFDeclContext _decl_ctx);
+  static DWARFDeclContext GetDWARFDeclContext(const DWARFDIE );
 
   static lldb::LanguageType LanguageTypeFromDWARF(uint64_t val);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2959,8 +2959,8 @@
 }
 
 if (try_resolving_type) {
-  DWARFDeclContext type_dwarf_decl_ctx;
-  GetDWARFDeclContext(type_die, type_dwarf_decl_ctx);
+  DWARFDeclContext type_dwarf_decl_ctx =
+  GetDWARFDeclContext(type_die);
 
   if (log) {
 GetObjectFile()->GetModule()->LogMessage(
@@ -3377,12 +3377,10 @@
 // declaration context.
 if ((parent_tag == DW_TAG_compile_unit ||
  parent_tag == DW_TAG_partial_unit) &&
-Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU( {
-  DWARFDeclContext decl_ctx;
-
-  GetDWARFDeclContext(die, decl_ctx);
-  mangled = decl_ctx.GetQualifiedNameAsConstString().GetCString();
-}
+Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(
+  mangled = GetDWARFDeclContext(die)
+.GetQualifiedNameAsConstString()
+.GetCString();
   }
 
   if (tag == DW_TAG_formal_parameter)
@@ -3984,14 +3982,13 @@
   return CompilerDeclContext();
 }
 
-void SymbolFileDWARF::GetDWARFDeclContext(const DWARFDIE ,
-  DWARFDeclContext _decl_ctx) {
-  if (!die.IsValid()) {
-dwarf_decl_ctx.Clear();
-return;
-  }
+DWARFDeclContext SymbolFileDWARF::GetDWARFDeclContext(const DWARFDIE ) {
+  if (!die.IsValid())
+return {};
+  DWARFDeclContext dwarf_decl_ctx =
+  die.GetDIE()->GetDWARFDeclContext(die.GetCU());
   dwarf_decl_ctx.SetLanguage(GetLanguage(*die.GetCU()));
-  die.GetDIE()->GetDWARFDeclContext(die.GetCU(), dwarf_decl_ctx);
+  return dwarf_decl_ctx;
 }
 
 LanguageType SymbolFileDWARF::LanguageTypeFromDWARF(uint64_t val) {
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -158,8 +158,7 @@
 return HasChildren() ? this + 1 : nullptr;
   }
 
-  void GetDWARFDeclContext(DWARFUnit *cu,
-   DWARFDeclContext _decl_ctx) const;
+  DWARFDeclContext GetDWARFDeclContext(DWARFUnit *cu) const;
 
   DWARFDIE GetParentDeclContextDIE(DWARFUnit *cu) const;
   DWARFDIE GetParentDeclContextDIE(DWARFUnit *cu,
@@ -169,6 +168,9 @@
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
 protected:
+  static DWARFDeclContext
+  GetDWARFDeclContextStatic(const DWARFDebugInfoEntry *die, DWARFUnit *cu);
+
   dw_offset_t m_offset; // Offset within the .debug_info/.debug_types
   uint32_t m_parent_idx; // How many to subtract from "this" to get the parent.
  // If zero this die has no parent
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -868,21 +868,30 @@
   }
 }
 
-void DWARFDebugInfoEntry::GetDWARFDeclContext(
-DWARFUnit *cu, DWARFDeclContext _decl_ctx) const {
-  const dw_tag_t tag = Tag();
-  if (tag == DW_TAG_compile_unit || tag == DW_TAG_partial_unit)
-return;
-  dwarf_decl_ctx.AppendDeclContext(tag, GetName(cu));
-  DWARFDIE parent_decl_ctx_die = GetParentDeclContextDIE(cu);
-  if (parent_decl_ctx_die && parent_decl_ctx_die.GetDIE() != this) {
-if (parent_decl_ctx_die.Tag() != DW_TAG_compile_unit &&
-

[Lldb-commits] [PATCH] D73787: [NFC] Refactor `GetDWARFDeclContext` to return `DWARFDeclContext`

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D73787#1859754 , @labath wrote:

> One way to this could be improved further is to change the recursion in 
> `GetDWARFDeclContext` into a loop (then you wouldn't need the extra wrapper).


True, I am stupid. But then I find there a need for the static method (as 
having `this` and iterating `die` pointer is error-prone).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73787



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

[Lldb-commits] [PATCH] D73787: [NFC] Refactor `GetDWARFDeclContext` to return `DWARFDeclContext`

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Yes, I agree this is better.

One way to this could be improved further is to change the recursion in 
`GetDWARFDeclContext` into a loop (then you wouldn't need the extra wrapper).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73787



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

[Lldb-commits] [PATCH] D73787: [NFC] Refactor `GetDWARFDeclContext` to return `DWARFDeclContext`

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added a reviewer: labath.
jankratochvil added a project: LLDB.
Herald added a subscriber: aprantl.
Herald added a reviewer: shafik.

In D70646  I broke 
http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/7400/ so this is a 
second attempt.
Unfortunately I do not have reproducible the breakage of Green Dragon locally.
The bug was that `DWARFDebugInfoEntry::GetDWARFDeclContext` reversed order of 
the items, that is fixed now.
OK for check-in? It is no longer so pretty but still worth it IMO.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73787

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

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -315,8 +315,7 @@
   static lldb_private::CompilerDeclContext
   GetContainingDeclContext(const DWARFDIE );
 
-  static void GetDWARFDeclContext(const DWARFDIE ,
-  DWARFDeclContext _decl_ctx);
+  static DWARFDeclContext GetDWARFDeclContext(const DWARFDIE );
 
   static lldb::LanguageType LanguageTypeFromDWARF(uint64_t val);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2956,8 +2956,8 @@
 }
 
 if (try_resolving_type) {
-  DWARFDeclContext type_dwarf_decl_ctx;
-  GetDWARFDeclContext(type_die, type_dwarf_decl_ctx);
+  DWARFDeclContext type_dwarf_decl_ctx =
+  GetDWARFDeclContext(type_die);
 
   if (log) {
 GetObjectFile()->GetModule()->LogMessage(
@@ -3374,12 +3374,10 @@
 // declaration context.
 if ((parent_tag == DW_TAG_compile_unit ||
  parent_tag == DW_TAG_partial_unit) &&
-Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU( {
-  DWARFDeclContext decl_ctx;
-
-  GetDWARFDeclContext(die, decl_ctx);
-  mangled = decl_ctx.GetQualifiedNameAsConstString().GetCString();
-}
+Language::LanguageIsCPlusPlus(GetLanguage(*die.GetCU(
+  mangled = GetDWARFDeclContext(die)
+.GetQualifiedNameAsConstString()
+.GetCString();
   }
 
   if (tag == DW_TAG_formal_parameter)
@@ -3981,14 +3979,13 @@
   return CompilerDeclContext();
 }
 
-void SymbolFileDWARF::GetDWARFDeclContext(const DWARFDIE ,
-  DWARFDeclContext _decl_ctx) {
-  if (!die.IsValid()) {
-dwarf_decl_ctx.Clear();
-return;
-  }
+DWARFDeclContext SymbolFileDWARF::GetDWARFDeclContext(const DWARFDIE ) {
+  if (!die.IsValid())
+return {};
+  DWARFDeclContext dwarf_decl_ctx =
+  die.GetDIE()->GetDWARFDeclContext(die.GetCU());
   dwarf_decl_ctx.SetLanguage(GetLanguage(*die.GetCU()));
-  die.GetDIE()->GetDWARFDeclContext(die.GetCU(), dwarf_decl_ctx);
+  return dwarf_decl_ctx;
 }
 
 LanguageType SymbolFileDWARF::LanguageTypeFromDWARF(uint64_t val) {
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -158,8 +158,7 @@
 return HasChildren() ? this + 1 : nullptr;
   }
 
-  void GetDWARFDeclContext(DWARFUnit *cu,
-   DWARFDeclContext _decl_ctx) const;
+  DWARFDeclContext GetDWARFDeclContext(DWARFUnit *cu) const;
 
   DWARFDIE GetParentDeclContextDIE(DWARFUnit *cu) const;
   DWARFDIE GetParentDeclContextDIE(DWARFUnit *cu,
@@ -169,6 +168,9 @@
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
 protected:
+  void DWARFDebugInfoEntry::GetDWARFDeclContext(
+  DWARFUnit *cu, DWARFDeclContext _decl_ctx) const;
+
   dw_offset_t m_offset; // Offset within the .debug_info/.debug_types
   uint32_t m_parent_idx; // How many to subtract from "this" to get the parent.
  // If zero this die has no parent
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -883,6 +883,12 @@
   }
 }
 
+DWARFDeclContext DWARFDebugInfoEntry::GetDWARFDeclContext(DWARFUnit *cu)