aprantl added a comment.
Very cool. I think we can improve the code quite a bit while we're here.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2427
if (attributes.ExtractFormValueAtIndex(i, form_value)) {
+ // AT_data_member_location indicates the byte offset of the
+ // word from the base address of the structure.
----------------
`DW_AT_data_member_location`.
`AT_foo` makes git grep less useful
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2674
- if (anon_field_info.IsValid()) {
+ if (unnamed_field_info.IsValid()) {
+ if (data_bit_offset != UINT64_MAX)
----------------
Can you please add comments explaining what each of these cases handle?
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2691
+ } else
+ field_bit_offset = this_field_info.bit_offset;
}
----------------
Looks like this could benefit from a refactoring with early exits. Can be a
follow-up commit though.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2696
+ last_bitfield_info = this_field_info;
+ } else {
+ last_bitfield_info.Clear();
----------------
for example, it's not obvious what happened to end up in this else branch
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:174
+ struct FieldInfo {
uint64_t bit_size;
uint64_t bit_offset;
----------------
Not you fault, but: This data structure is a disaster waiting to happen.
Instead of having magic sentinel values, should we use an Optional<FieldInfo>
that is always valid if it exists? Or should the members be Optionals?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72953/new/
https://reviews.llvm.org/D72953
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
