[PATCH] [yaml2obj] ELF Relocations Support

[Sean Silva]

  Nice job reducing the Rel/Rela duplication!

  On a second look I have a some small questions about the tag handling.


================
Comment at: tools/yaml2obj/yaml2elf.cpp:245
@@ -236,1 +244,3 @@
+        return false;
+        ;
       }
----------------
Stray semicolon?

================
Comment at: tools/yaml2obj/yaml2elf.cpp:268-269
@@ +267,4 @@
+    } else {
+      errs() << "error: Unknown section type.\n";
+      return false;
+    }
----------------
This is actually unreachable, right? If we get here then someone has extended the ELFYAML::Section hierarchy without adding a case here which I think is a programming error.

================
Comment at: lib/Object/ELFYAML.cpp:582-591
@@ +581,12 @@
+    } else if (auto S = dyn_cast<ELFYAML::RelocationSection>(Section.get())) {
+      IO.mapTag("!Relocation", true);
+      MappingTraits<ELFYAML::RelocationSection>::mapping(IO, *S);
+    } else
+      IO.setError("Unknown section type");
+  } else {
+    if (IO.mapTag("!Section", true) || IO.mapTag("tag:yaml.org,2002:map")) {
+      Section.reset(new ELFYAML::ContentSection());
+      MappingTraits<ELFYAML::ContentSection>::mapping(
+          IO, static_cast<ELFYAML::ContentSection &>(*Section));
+    } else if (IO.mapTag("!Relocations")) {
+      Section.reset(new ELFYAML::RelocationSection());
----------------
Is there a reason that on one side there is "!Relocation" and on the other "!Relocations" (plural)?

================
Comment at: test/Object/yaml2obj-elf-rel.yaml:40
@@ +39,3 @@
+  Relocations:
+    - !Relocation
+      Offset: 0x1
----------------
Is this per-relocation "!Relocation" tag needed? Doesn't the "!Relocations" on the section cover this?


http://reviews.llvm.org/D3302