[PATCH] [yaml2obj] ELF Relocations Support

Sean Silva chisophugis at gmail.com
Wed Apr 9 11:35:48 PDT 2014



================
Comment at: lib/Object/ELFYAML.cpp:579-580
@@ +578,4 @@
+    if (auto S = dyn_cast<ELFYAML::ContentSection>(Section.get())) {
+      IO.mapTag("!Section", true);
+      MappingTraits<ELFYAML::ContentSection>::mapping(IO, *S);
+    } else if (auto S = dyn_cast<ELFYAML::RelocationSection>(Section.get())) {
----------------
Simon Atanasyan wrote:
> Sean Silva wrote:
> > Now that I think about it, since in the future the ELFYAML::Section hierarchy is going to be growing, I think it would make sense to standardize on a consistent tag name scheme. It just bothers me that the generic "!Section" is being used for ContentSection, while "!Relocations" for RelocationSection. Can we standardize on having the tag be the name of the class?
> Agreed. But I am not sure that ContentSection is a good name for tag. What do you think?
Now that I think more about it, I think that the tag system is just overcomplicating things. I think it would make a lot more sense and be simpler to just autodetect which ELFYAML::Section subclass to create based on which keys are present in the section entry.

i.e., the deserialization deserializes `Content:` *and* `Relocations:` (and any future additions) as optional, then chooses which subclass to instantiate based on what is present. Of course, it issues an error if there is an illegal combination of keys.

The idea is that `Content:`, `Relocations:`, etc. (any future ones) would be just sugar for filling in the `Content:` in a more convenient/readable way; this approach makes the whole process easier to understand from a user's perspective.

I think the goal of the tags (making it a bit more "strongly typed" on disk) is admirable, but at the end of the day we are still duck typing (from a tag perspective) between Rel and Rela (and probably a bunch of other things). It's also just plain redundant.


http://reviews.llvm.org/D3302






More information about the llvm-commits mailing list