[Lldb-commits] [PATCH] D84008: [DWARFYAML][WIP] Refactor emitDebugInfo() to make the length be inferred.

[Xing GUO via Phabricator via lldb-commits]

Higuoxing added a comment.

In D84008#2161461 <https://reviews.llvm.org/D84008#2161461>, @labath wrote:

> In D84008#2161243 <https://reviews.llvm.org/D84008#2161243>, @Higuoxing wrote:
>
> > In D84008#2160426 <https://reviews.llvm.org/D84008#2160426>, @MaskRay wrote:
> >
> > > The number of changed tests is large. Is it worth moving the `IO.mapOptional("Length", Unit.Length);` change to a separate patch to make the refactoring more focused? Thanks
> >
> >
> > This patch is intended to make the length field be inferred when emitting the .debug_info section. If we move the `IO.mapOption("Length", Unit.Length);` change to a separate change, we might not be able to know when to infer the length? There are two visitors, `DumpVisitor` which is used to emit the .debug_info section and `DIEFixupVisitor` which is used to calculate the length field for us. Do you mean that we keep the `DIEFixupVisitor` class and remove the `DumpVisitor` class in this patch?
>
>
> I think that should work if you make it so that this other patch comes before the functional change in this patch. That other patch could change the encoding to hex (`uint64_t Length;` -> `yaml::Hex64 Length;`) and make it default to zero (`IO.mapRequired("Length", Unit.Length);` -> `IO.mapOptional("Length", Unit.Length, 0);`). That should have no functional change (I think), but allow you to make the changes in all the yaml files. The visitation stuff could come after that.


Thank you @labath, It sounds good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84008