This is an archive of the discontinued LLVM Phabricator instance.

WebAssembly: remove 'external' from test
ClosedPublic

Authored by jfb on Dec 23 2015, 11:01 AM.

Details

Summary

Linker testing was sad at seeing an unresolved external symbol. For now don't do that: it's valid but we're not playing with multi-file linking yet, and the LLVM tests are used as hacky sanity tests for single-file linking (the GCC torture tests are much better for this purpose). Another solution would be to use '.extern' to make the intent explicit (don't simple-file link this, there's an unresolved symbol), some assemblers use '.extern' while others ignore it, so we wouldn't really be inventing anything new.

Diff Detail

Event Timeline

jfb updated this revision to Diff 43549.Dec 23 2015, 11:01 AM
jfb retitled this revision from to WebAssembly: remove 'external' from test.
jfb updated this object.
jfb added reviewers: sunfish, kripken.
jfb added a subscriber: llvm-commits.
sunfish accepted this revision.Dec 23 2015, 3:39 PM
sunfish edited edge metadata.

As we discussed on IRC, this test was using external intentionally, because that's a useful thing to test, and the problem is that these tests are being use to generate tests for other tools in which external doesn't make sense. I'm ok with this change as a workaround for now, but please add a TODO comment reminding us to go back and revisit this.

This revision is now accepted and ready to land.Dec 23 2015, 3:39 PM
jfb updated this revision to Diff 43563.Dec 23 2015, 3:58 PM
jfb edited edge metadata.
  • Add fixme.
This revision was automatically updated to reflect the committed changes.