[PATCH] D101341: Initialize optional members of ELFYAML types.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D101341#2755605 <https://reviews.llvm.org/D101341#2755605>, @vzakhari wrote:

> In D101341#2753240 <https://reviews.llvm.org/D101341#2753240>, @jhenderson wrote:
>
>> @vzakhari, what's the motivation for doing this? These data structures are used within yaml2obj/obj2yaml, although I'm not particularly familiar with the plumbing to identify whether it matters. My suspicion is that it doesn't.
>
> @jhenderson, there are two ways to create a YAML file using YAML mappings.  The first one is to create a YAML file from a string bufffer (see line 38 in `llvm/unittests/ObjectYAML/ELFYAMLTest.cpp`).  The second way is to populate the `ELFYAML::` data structures and stream them into `yaml::Output`.  Both ways are valid.  The second way is a little bit more convenient for programmatic ELF-YAML creation in my downstream application.  When I try to use `ELFYAML::` data structures, there is no convenient way to make sure that all optional members in them are initialized.  Neither default nor zero-initialization works.  This is why I made these changes to make sure that all optional members are initialized properly.  Note that the default values are not necessarily zeroes.

Which cases don't have zero as their default value? The ones you've modified in this patch all are (even the Mips enums are - the enum values are value 0).

Frankly speaking, I don't like the style of the change: it looks somewhat ugly (`= 0` would be better than `{0}` in my opinion at least), but more importantly there's no guarantee that it will continue to work - more fields could be added at a later date, for example, without this initialisation, especially as it's not clear when it needs to be explicitly initialised (this change for example doesn't change the `Size` field of the `StackSizeEntry` struct). It sounds like there's no internal-to-LLVM usage of this approach, so it could easily be considered unnecessary and get dropped at a later date too.


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

https://reviews.llvm.org/D101341