[PATCH] D75123: [obj2yaml, yaml2obj] - Read and dump the "Content" key of the RawContentSection section as array.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D75123#1897585 <https://reviews.llvm.org/D75123#1897585>, @grimar wrote:

> I am not sure why we might want strings. Usually we describe a binary context of
>  sections isn't? e,g. custom symbol tables, custom text sections (with broken instructions) for broken test cases etc.


I just feel that the current "3031" (hex pairs) representation is not conventional. C string literals may meet users' intuition.

In D75123#1900469 <https://reviews.llvm.org/D75123#1900469>, @jhenderson wrote:

> In D75123#1897851 <https://reviews.llvm.org/D75123#1897851>, @grimar wrote:
>
> > There are cases where using yaml2obj is preferable over llvm-mc, but you may have blobs that are not little. Imagine you prototype a new section and want to write a test
> >  for a dumper tool for example. It is not always reasonable to teach yaml2obj about a new section type. So you still might want to use `Content`, but want to comment bytes well.
> >  It is currently impossible to split `Content` data to leave multi-line comments.
>
>
> FWIW, I actually disagree with the statement that it is not always reasonable to teach yaml2obj about a new section type. I think if a binary blob is large enough that it needs commenting, it should be added to yaml2obj instead.
>
> That being said, I could imagine a limited set of cases involving malformed inputs where even if the section is implemented in yaml2obj, you might need to use raw `Content`.
>
> > An idea:
> >  What if we introduce a one more YAML key: "ContentBytes"? I.e. we keep `Optional<yaml::BinaryRef> Content` as is,
> >  but also add `Optional<std::vector<uint8_t>> ContentBytes` for those who would like to comment group of bytes
> >  and use `[ 0x1, 0x2 ]` style for whatever reason is.
>
> This could possibly work, but I suspect people might be confused as to when to use one instead of the other. We probably need to update the `yaml2obj`/`obj2yaml` documentation. I might also suggest `ContentArray` as a name instead perhaps?


I don't have a strong opinion. `ContentArray` looks fine. obj2yaml does not know the nature of the content (strings or blob), should it continue dumping to `Content` by default? (I'll vote for yes; no strong opinion) If yes, `ContentArray` will be a key only known by yaml2obj.


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

https://reviews.llvm.org/D75123