[PATCH] D80311: [lld][test] Expand testing for dynamic-list and export-dynamic

[Owen Reynolds via Phabricator via llvm-commits]

gbreynoo added a comment.

Thanks for the comments Maskray. I had some questions regarding the comments for expanding the test for use of -shared and the similarities with dynsym-pie.s, symbols.s and visibility.s. My intention with the test export-symbol-types.s was to confirm the  symbols types that are placed in the dynamic symbol table with use of --export-dynamic and --dynamic-list. I considered the other tests mentioned as covering related but different functionality using similar methods.  For example symbol.s being for correct symbol output as a whole, not specifically  exports etc.

I can see there being a reason to expand the coverage of the existing test to cover -shared, this would require the rename and potentially splitting the existing test in two. It may also be worth creating one input assembly file that can be used for all these tests mentioned to ensure coverage. Could this wait for a future change however?

Also, with your comment were you suggesting removing some of the existing tests and folding them into the one added here? As they test different functionality I think it may be best to keep them separate.


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

https://reviews.llvm.org/D80311