[PATCH] D69709: [yaml2obj] - Add a way to describe the custom data that is not part of an output section.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 6 00:14:21 PST 2019


grimar marked an inline comment as done.
grimar added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:1485
   IO.mapOptional("ProgramHeaders", Object.ProgramHeaders);
-  IO.mapOptional("Sections", Object.Sections);
+  IO.mapOptional("Sections", Object.Descriptions);
   IO.mapOptional("Symbols", Object.Symbols);
----------------
MaskRay wrote:
> I feel that using `Sections` for `Fragment`s may be confusing because `Fragment`s are not described by the section header table. Does a new field add a lot of complexity? If not, we can add a parallel YAML mapping key `Fragments`, and use `Fragments` instead of `Sections` when there is a need for a `Fragment` (i.e. when there is non-section data that needs describing).
> 
> Opinions? @rupprecht @jhenderson @labath
To clarify I understood the idea: do you mean that YAML writers should be able to use both "Sections" and "Fragments", where the latter will be just an alias for "Sections"? I.e to make both below to be valid?

```
Fragments:
- Section1
- Fragmen1
- Section2
```

```
Sections:
- Section1
- Fragmen1
- Section2
```

With that since they are both optional we will need to add a check that they are not specified at the same time. Also, I guess most of users will just randomly use either "Sections" or "Fragments" because of common copy-paste approach. I'd prefer just to rename "Sections" to something if there are any concerns about the naming.

When I wrote this patch I thought about this question though and came up to thinking about this part of YAML just like about a linker script's "SECTIONS" command. It might contain not only sections, but location counter moving, commands like "ASSERT" e.t.c, but it is still called "SECTIONS" and I probably do not find it confusing.


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

https://reviews.llvm.org/D69709





More information about the llvm-commits mailing list