[PATCH] D39582: Add ELF dynamic symbol support to yaml2obj/obj2yaml

[Dave Lee via Phabricator via llvm-commits]

kastiglione added inline comments.


================
Comment at: tools/yaml2obj/yaml2elf.cpp:301
   zero(SHeader);
-  SHeader.sh_name = DotShStrtab.getOffset(".symtab");
-  SHeader.sh_type = ELF::SHT_SYMTAB;
-  SHeader.sh_link = getDotStrTabSecNo();
+  bool IsStatic = STType == SymtabType::Static ? true : false;
+  SHeader.sh_name = DotShStrtab.getOffset(IsStatic ? ".symtab" : ".dynsym");
----------------
jakehehrlich wrote:
> kastiglione wrote:
> > jakehehrlich wrote:
> > > Can this just be "bool IsStatic = STType == SymtabType::Static"? Better yet do we really need SymtabType? What if we just pass "IsStatic" as a parameter?
> > 🤦‍♂️
> > 
> > The choice to use the enum was to avoid opaque `true`/`false` arguments at the call site. I'll go with bools if that's preferred.
> I'm not sure I have a preference on bools vs enums, you pick, but the ternary operator use seems funny to me.
Agreed, I got rid of this ternary right away, just haven't update the diff.


https://reviews.llvm.org/D39582