[PATCH] D37391: [yaml2obj][ELF] Add support for symbol indexes greater than SHN_LORESERVE

[Roland McGrath via Phabricator via llvm-commits]

mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

Not being expert in the yaml2obj code base, this looks good to me with the caveats below.



================
Comment at: include/llvm/ObjectYAML/ELFYAML.h:53
 LLVM_YAML_STRONG_TYPEDEF(uint64_t, ELF_SHF)
+LLVM_YAML_STRONG_TYPEDEF(uint32_t, ELF_SHN)
 LLVM_YAML_STRONG_TYPEDEF(uint8_t, ELF_STT)
----------------
This is actually a 16-bit field in ElfNN_Sym.



================
Comment at: lib/ObjectYAML/ELFYAML.cpp:743
+  }
+  if (Symbol.Index && Symbol.Index < ELFYAML::ELF_SHN(ELF::SHN_LORESERVE)) {
+    return "Use a section name to define which section a symbol is defined in";
----------------
It looks like llvm::Optional has both operator* and operator==/operator< et al that essentially forward the operator to the value that operator* would yield (in the has-value case, anyway--the overall semantics of the comparison operators on Optional are a bit weird, and I think there's some implicit copy construction in there too), so either Symbol.Index or *Symbol.Index works here, but you should be consistent, and *Symbol.Index looks more natural next to the Symbol.Index->bool test (and I suspect is the only pattern with sensible semantics when dealing with Optional<T> where T is not a trivial POD type like this one).


Repository:
  rL LLVM

https://reviews.llvm.org/D37391