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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo marked 8 inline comments as done.
mstorsjo added inline comments.


================
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
----------------
jhenderson wrote:
> Are all these required? Same comment for the other sections.
Yes, `Characteristics` is mandatory, otherwise it fails like this:

```
YAML:33:5: error: missing required key 'Characteristics'
```


================
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
----------------
jhenderson wrote:
> Are these fields required? Same question for further below.
Yes, these fields are necessary.


================
Comment at: test/tools/yaml2obj/coff-symbol-index.yaml:78
+      NumberOfLinenumbers: 0
+      CheckSum:        1996959894
+      Number:          3
----------------
jhenderson wrote:
> Is this necessary?
The whole `SectionDefinition` part can be omitted while keeping the test functioning.


================
Comment at: test/tools/yaml2obj/coff-symbol-index.yaml:80
+      Number:          3
+      Selection:       IMAGE_COMDAT_SELECT_ANY
+  - Name:            foo
----------------
jhenderson wrote:
> Is this needed?
Can be omitted while keeping the test working.


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

https://reviews.llvm.org/D56294