labath added a comment.
This seems fairly straight-forward to me, though I didn't dive into all the
details.
Given the amount of changed tests, it may be nice to separate the format change
from the linking patch (so in the interim state, one could generate multiple
abbrev tables, but there would be no (reasonable) way to reference any table
except the first one).
================
Comment at: llvm/include/llvm/ObjectYAML/DWARFEmitter.h:34
+private:
+ Data &DWARF;
+ std::map<uint64_t, uint64_t> AbbrevID2Index;
----------------
jhenderson wrote:
> Would it make any sense to merge the `DWARFYAML::Data` class and
> `DWARFYAML::DWARFState` class at all?
That would definitely be nice.
================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:89
+DWARFYAML::DWARFState::DWARFState(DWARFYAML::Data &DI, Error &Err) : DWARF(DI)
{
+ ErrorAsOutParameter EAO(&Err);
+
----------------
If you'd do all this work in the factory function and then just pass in a
finished map to the constructor, there'd be no need for the
`ErrorAsOutParameter` thingy.
================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:91
+
+ for (uint64_t I = 0; I < DI.DebugAbbrev.size(); ++I) {
+ // If the abbrev table's ID isn't specified, we use the index as its ID.
----------------
consider: `for (auto &Abbr : enumerate(DI.DebugAbbrev))`
================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:93-95
+ uint64_t ID = I;
+ if (DI.DebugAbbrev[I].ID)
+ ID = *DI.DebugAbbrev[I].ID;
----------------
`ID = DI.DebugAbbrev[I].ID.getValueOr(I)`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83116/new/
https://reviews.llvm.org/D83116
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
