shafik accepted this revision.
shafik added a comment.
LGTM with minor comments. Thank you for these fixes, they are awesome!
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3213
+ bool use_type_size_for_value = false;
+ if (location_form.IsValid()) {
+ has_explicit_location = true;
----------------
It might be helpful to document that `DW_AT_location` is taken over
`DW_AT_const_value` and the types of cases this can show up in.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3216
+ if (DWARFFormValue::IsBlockForm(location_form.Form())) {
+ auto data = die.GetData();
+
----------------
`const DWARFDataExtractor&`?
I don't think `auto` adds any value here.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3241
+ // string.
+ auto debug_info_data = die.GetData();
+ if (DWARFFormValue::IsBlockForm(const_value_form.Form())) {
----------------
`const DWARFDataExtractor&`?
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3257
+ } else if (const char *str = const_value_form.AsCString()) {
+ uint32_t string_length = strlen(str) + 1;
+ location = DWARFExpression(
----------------
aprantl wrote:
> If we do this a lot a StringRef DWARFFormValue::AsCStringRef() call would
> make sense...
Why `+1`?
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3446
SymbolFileTypeSP type_sp(
new SymbolFileType(*this, GetUID(type_die_form.Reference())));
----------------
`make_shared`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86615/new/
https://reviews.llvm.org/D86615
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
