[PATCH] D68943: [llvm][yaml2obj] no more implicit ELF .symtab section

[Konrad Wilhelm Kleine via Phabricator via llvm-commits]

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


================
Comment at: llvm/include/llvm/ObjectYAML/ELFYAML.h:379-380
   // being a single SHT_SYMTAB section are upheld.
-  std::vector<Symbol> Symbols;
+  Optional<std::vector<Symbol>> Symbols;
   std::vector<Symbol> DynamicSymbols;
 };
----------------
labath wrote:
> This creates an inconsistency between how .symtab and .dynsym sections are handled, which is hard to justify. I actually prefer the Optional<> version, because that enables one to say "the symtab section is there, but it is _empty_", something which is impossible with the way .dynsym emission works (the section gets surpressed if it is empty), but the question is, shouldn't then the .dynsym emission work the same way ? Have you looked at by any chance how hard it would be to change .dynsym to follow the same pattern?
> 
> (In any case, I think this is up to yaml2obj maintainers to decide how to handle this. I'll add some to this review.)
@labath I haven't looked but will investigate how hard it is to declare `Optional<std::vector<Symbol>> DynamicSymbols;`. That's what I understood at least.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943