[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