[PATCH] D63487: [yaml2obj/obj2yaml] - Make RawContentSection::Info to be Optional<>

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Looks good aside from some minor comments. By the way, you don't need the "to be" in the patch title.



================
Comment at: test/tools/obj2yaml/implicit-sections-info.yaml:1
+## Check that obj2yaml does not create Info field in
+## the case when it has a value of zero.
----------------
I'm not sure this test needs to be called "implicit-sections-info.yaml". Simply "section-info.yaml" seems like enough.


================
Comment at: test/tools/obj2yaml/implicit-sections-info.yaml:1
+## Check that obj2yaml does not create Info field in
+## the case when it has a value of zero.
----------------
jhenderson wrote:
> I'm not sure this test needs to be called "implicit-sections-info.yaml". Simply "section-info.yaml" seems like enough.
create Info -> write a section Info


================
Comment at: test/tools/obj2yaml/implicit-sections-info.yaml:23-24
+    Type: SHT_PROGBITS
+  - Name:  .bar
+    Type:  SHT_PROGBITS
+    Info: 1
----------------
Nit: spurious extra spaces between tag and value on these two lines.


================
Comment at: tools/yaml2obj/yaml2elf.cpp:490
     SHeader.sh_entsize = *YAMLSec->EntSize;
+  if (RawSec && RawSec->Info)
+    SHeader.sh_info = *RawSec->Info;
----------------
There should probably be a blank line before this if, to match the other blocks.


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

https://reviews.llvm.org/D63487