labath added a comment.
I like this patch in a lot of ways, because it makes the code cleaner, and
prepares us for DWARF5, which can reference entries in other files in a lot of
interesting ways.
The part I am not totally sure if is the alignment of lldb's DWARFFormValue
with it's llvm counterpart. The llvm version does not have this fancy
cross-unit resolution capability, but it just returns the raw value and leaves
it up to the user to make sense of it (much like our class does now). However,
it's possible that the llvm version does not have this functionality because
there wasn't need for it, and it already has access to the DWARFContext, so it
is certainly prepared to do more complex lookups, so it's possible it could be
extended similarly once we go around to merging the two. And even if we go back
to the world where `form.Reference()` returns a raw value, this may serve as a
good intermediate step, because otherwise we'd have a hard time catching all
the cases where someone calls `Reference`, and expects it to return an simple
section offset.
So, overall, I'm inclined to accept this patch, but I'd like to hear what
@JDevlieghere and @clayborg think.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp:114-115
DWARFDIE
DWARFDIE::GetReferencedDIE(const dw_attr_t attr) const {
- const dw_offset_t die_offset =
- GetAttributeValueAsReference(attr, DW_INVALID_OFFSET);
- if (die_offset != DW_INVALID_OFFSET)
- return GetDIE(die_offset);
- else
- return DWARFDIE();
+ return GetAttributeValueAsReference(attr);
}
----------------
It looks like these two functions are equivalent now and we can remove one of
them (I vote to keep this one).
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61502/new/
https://reviews.llvm.org/D61502
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
