[PATCH] D95140: [yaml2obj, obj2yaml] - Implement section header table as a special Chunk.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:1840
+  const ELFYAML::SectionHeaderTable &SectionHeaders = Doc.getSectionHeaderTable();
+  if (!SectionHeaders.IsImplicit) {
+    if (SectionHeaders.Excluded)
----------------
jhenderson wrote:
> Do you actually need the `IsImplicit` check here? The default values of NoHeaders and Excluded should be such that the ifs inside don't trigger anyway.
Right. Removed.


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:1972
+  const ELFYAML::SectionHeaderTable &SHT = Doc.getSectionHeaderTable();
+  if (SHT.Offset)
+    CBA.updateDataAt(*SHT.Offset, SHeaders.data(),
----------------
jhenderson wrote:
> I think it would be more self-documenting if this was `if(!SHT.NoHeaders)`. If I understand it correctly, SHT.Offset will always be set when NoHeaders is false. If NoHeaders is false, specifying Offset explicitly doesn't make sense, and is prohibited.
Done.


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

https://reviews.llvm.org/D95140