[PATCH] D56294: [ObjectYAML] [COFF] Support multiple symbols with the same name

Mon Jan 7 04:38:47 PST 2019

jhenderson accepted this revision.
jhenderson added a comment.

Okay, looking much better. I'm happy for this to go in, but if any of the other bits I highlighted can be removed, even better.

================
Comment at: test/tools/yaml2obj/coff-symbol-index.yaml:34
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
----------------
Are all these required? Same comment for the other sections.

================
Comment at: test/tools/yaml2obj/coff-symbol-index.yaml:59-61
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
----------------
Are these fields required? Same question for further below.

================
Comment at: test/tools/yaml2obj/coff-symbol-index.yaml:78
+      NumberOfLinenumbers: 0
+      CheckSum:        1996959894
+      Number:          3
----------------
Is this necessary?

================
Comment at: test/tools/yaml2obj/coff-symbol-index.yaml:80
+      Number:          3
+      Selection:       IMAGE_COMDAT_SELECT_ANY
+  - Name:            foo
----------------
Is this needed?

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

https://reviews.llvm.org/D56294