[PATCH] D83116: [DWARFYAML] Add support for referencing different abbrev tables.

[Xing GUO via Phabricator via llvm-commits]

Higuoxing added a comment.





================
Comment at: llvm/include/llvm/ObjectYAML/DWARFEmitter.h:34
+private:
+  Data &DWARF;
+  std::map<uint64_t, uint64_t> AbbrevID2Index;
----------------
labath wrote:
> jhenderson wrote:
> > Would it make any sense to merge the `DWARFYAML::Data` class and `DWARFYAML::DWARFState` class at all?
> That would definitely be nice.
>>! @jhenderson :
> Would it make any sense to merge the `DWARFYAML::Data` class and `DWARFYAML::DWARFState` class at all?

>>! @labath :
> That would definitely be nice.

I think they can be merged. But is there any particular reason to merge them? Personally, I would like to keep the `DWARFYAML::DWARFState` class. Here are a few concerns I would like to address:

- If we merge the `DWARFYAML::DWARFState` and `DWARFYAML::Data` at all, we will need to add some helper variables, e.g. `AbbrevTableID2Index` and a method like `Error DWARFYAML::link()` to `DWARFYAML::Data` class to link DWARF sections. We will have to call this special method before DWARF sections being emitted. If we port DWARF support to other binary format in the future, e.g., wasm, we might forget to call this method. If we make DWARF emitters accept `DWARFYAML::DWARFState`, it's easy to figure out that we need a `DWARFState` before emitting DWARF sections.

- Besides, I think `DWARFYAML::Data` and `DWARFYAML::DWARFState` have different functions. `DWARFYAML::Data` is used to contain original DWARF section descriptions and `DWARFYAML::DWARFState` contains helper structs and methods which helps link DWARF sections. It might be good not to merge them so that we can easily figure out when and where are those sections get linked?

I can be persuaded :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83116/new/

https://reviews.llvm.org/D83116