labath added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84008/new/ https://reviews.llvm.org/D84008 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
