[PATCH] D73888: [yaml2obj][obj2yaml] - Simplify format of the SHT_LLVM_ADDRSIG section.
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 4 01:38:21 PST 2020
jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.
LGTM, once @MaskRay is happy.
================
Comment at: llvm/test/tools/llvm-readobj/ELF/addrsig.test:23
Type: SHT_LLVM_ADDRSIG
- Symbols:
- - Name: foo
- - Name: bar
+ Symbols: [ foo, bar ]
Symbols:
----------------
MaskRay wrote:
> MaskRay wrote:
> > grimar wrote:
> > > Thinking about his again, I wonder if we want the following output format:
> > > ```
> > > Symbols:
> > > - foo
> > > - bar
> > > ```
> > >
> > > All what is needed is to stop using `YAMLFlowString` and use `StringRef`,
> > > so the change is trivial.
> > > That might produce a nicer (?) output when symbol names are long probably,
> > > though I am not sure we should care too much here and/or what is better.
> > Is there a test having a special symbol name (a metacharacter in YAML.)? e.g. `[`
> >
> > Just want to make sure they are properly quoted.
> A list of `YAMLFlowString` can be wrapped (default wrap column is 70), can't it?
>
> If yes, I think `YAMLFlowString` is fine. I don't have a strong opinion, though. We can also use `StringRef`.
I personally prefer the `YAMLFlowString` syntax, as long as it can be wrapped as @MaskRay mentions. I think we made a similar change in recent months in that direction for something else, but I don't remember what.
However, if there's strong reasoning to favour the `StringRef` approach, I don't mind either.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73888/new/
https://reviews.llvm.org/D73888
More information about the llvm-commits
mailing list