[PATCH] D90301: [yaml2obj][test] - Merge dynsymtab-shlink.yaml to dynsym-section.yaml

Thu Oct 29 02:08:53 PDT 2020

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM.

================
Comment at: llvm/test/tools/yaml2obj/ELF/dynsym-section.yaml:60
+
+# RUN: yaml2obj %s --docnum=2 -DLINK="" -o %t3
+# RUN: llvm-readelf --section-headers %t3 | FileCheck %s --check-prefix=LINK-DEFAULT
----------------
grimar wrote:
> jhenderson wrote:
> > If you used Link: `[[LINK:<none>]]` (or whatever the syntax is), that'll allow you to avoid the `-DLINK=""` bit here, right?
> No, because `Link` field is not `Optional<>`, it is StringRef. I can't use `[[LINK:<none>]]` currently.
> Perhaps worth changing it to be `Optional` too.
Ah, that's a surprise. I'd expect a user not to understand the internal differences that prevent this, so changing it to be an `Optional` (in a future patch) would make sense to me.

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

https://reviews.llvm.org/D90301