[PATCH] D84526: [yaml2obj] - Add a support for "<none>" value for all optional fields.
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 28 04:19:29 PDT 2020
jhenderson added a comment.
In D84526#2178269 <https://reviews.llvm.org/D84526#2178269>, @grimar wrote:
> In D84526#2178235 <https://reviews.llvm.org/D84526#2178235>, @jhenderson wrote:
>
>> What's the motivation for testing every `Optional<T>` type? It seems to me like this code is a common piece of code where a single example would be sufficient.
>
> OK, probably you're right. What about having a full set for a section instead?
> E.g I'll compare:
>
> - Name: .bar
> Type: SHT_PROGBITS
> Offset: [[TEST=<none>]]
> Address: [[TEST=<none>]]
> Content: [[TEST=<none>]]
> Size: [[TEST=<none>]]
> ContentArray: [[TEST=<none>]]
> Info: [[TEST=<none>]]
> EntSize: [[TEST=<none>]]
> ShName: [[TEST=<none>]]
> ShOffset: [[TEST=<none>]]
> ShSize: [[TEST=<none>]]
> ShFlags: [[TEST=<none>]]
>
> vs
>
> - Name: .bar
> Type: SHT_PROGBITS
>
> The first set contains some possible interactions between `Content`, `Size`, `ContentArray` and `ShSize`,
> between `Name` and `ShName`, `Offset` and `ShOffset` etc. I.e. probably demonstrates
> the having `=<none>` really does nothing.
Yeah, that seems reasonable to me.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84526/new/
https://reviews.llvm.org/D84526
More information about the llvm-commits
mailing list