ayermolo marked 16 inline comments as done.
ayermolo added inline comments.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DIERef.h:32
enum Section : uint8_t { DebugInfo, DebugTypes };
-
- DIERef(std::optional<uint32_t> dwo_num, Section section,
+ // Making constructors protected to limit where DIERef is used directly.
+ DIERef(std::optional<uint32_t> file_index, Section section,
----------------
clayborg wrote:
> labath wrote:
> > they're not actually protected
> There were for a bit to control access to this, but in reality we ended up
> friending too many classes and then ran into issues with the DIERefTest
> stuff, so we decided to make them public again. We can remove this comment.
Oops, forgot to remove. For one of the internal revisions I experimented with
making them protected.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:198
"attach the file at the start of this error message",
- m_offset, (unsigned)form);
+ (uint64_t)m_offset, (unsigned)form);
*offset_ptr = m_offset;
----------------
clayborg wrote:
> Needed? Same as above
m_offset is is bit field now, so without it clang produces error.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1682
SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
- if (die_ref.dwo_num()) {
- SymbolFileDWARF *dwarf = *die_ref.dwo_num() == 0x3fffffff
- ? m_dwp_symfile.get()
- : this->DebugInfo()
- .GetUnitAtIndex(*die_ref.dwo_num())
- ->GetDwoSymbolFile();
- return dwarf->DebugInfo().GetDIE(die_ref);
- }
-
- return DebugInfo().GetDIE(die_ref);
+ return GetDIE(die_ref.get_id());
}
----------------
clayborg wrote:
> labath wrote:
> > clayborg wrote:
> > > Pavel: note we now really on "SymbolFileDWARF::GetDie(user_id_t)" to be
> > > the one source of truth when finding a DIE. We could make
> > > "SymbolFileDWARF:GetDie(DIERef ref)" be the one source of truth and then
> > > have "SymbolFileDWARF::GetDie(user_id_t)" just create a local DIERef and
> > > then call "SymbolFileDWARF:GetDie(DIERef ref)" if that would be cleaner.
> > +1
> Ok. So lets do this - change "DWARFDIE
> SymbolFileDWARF::GetDIE(lldb::user_id_t uid)" to just be:
>
> ```
> DWARFDIE SymbolFileDWARF::GetDIE(lldb::user_id_t uid) {
> return GetDIE(DIERef(uid));
> }
> ```
> And then change the current "DWARFDIE SymbolFileDWARF::GetDIE(lldb::user_id_t
> uid)" to be the one that does all of the work:
>
> ```
> DWARFDIE SymbolFileDWARF::GetDIE(DIERef die_ref) {
> std::optional<uint32_t> file_index = die_ref.file_index();
> if (file_index) {
> if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile())
> symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO
> case
> else if (*file_index == DIERef::k_file_index_mask)
> symbol_file = m_dwp_symfile.get(); // DWP case
> else
> symbol_file = this->DebugInfo()
> .GetUnitAtIndex(*die_ref.file_index())
> ->GetDwoSymbolFile(); // DWO case
> } else if (die_ref.die_offset() == DW_INVALID_OFFSET) {
> symbol_file = nullptr;
> } else {
> symbol_file = this;
> }
>
> if (symbol_file)
> return symbol_file->GetDIE(die_ref);
>
> return DWARFDIE();
> }
> ```
>
ah, yes, great suggestion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138618/new/
https://reviews.llvm.org/D138618
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
