alexcrichton commented on issue #6610:
I realize after writing that all out that you've probably already also considered such an idea, so I could also rephrase much of it as "I'm not sure why a copy of
block.wast
was added", but if that's well-motivated them seems fine!
saulecabrera commented on issue #6610:
I'm not sure why a copy of block.wast was added", but if that's well-motivated them seems fine!
Yeah, the copy of the spec tests is one of the unfortunate bits of this approach. Unfortunately attempting to run all the tests and adding
#[ignore]
when a non-supported feature is detected will result in most (if not all) of the official spec tests getting ignored, since as far as I can tell, there's no way to ignore spec tests at a more granular level. Even though Winch is shaping up, it doesn't offer support for finished proposals that are already part of the official spec suite, like multi-value for example, which many of the tests in the official spec test suite use. But it's totally possible that I might have missed a better way of avoiding the duplication, and if that's the case, I'm all in to explore alternatives!
alexcrichton commented on issue #6610:
One idea perhaps would be to add a test for
strategy
in theignore
function against"Winch"
and blanket returntrue
(ignore everything) and then add a small allow-list for tests there? Or alternative blanket-ignore entire directories/subtrees but blanket-include others, for exampletests/misc_testsuite/winch/*.wast
could be tested in winch for now?Otherwise though this is a transient state of affairs while support is shored up in winch so while I personally try to minimize the churn during the transient state I don't think it's bad to have churn and/or copies of files, so again I think whatever's easiest for your development should work best. In the long run everything should be run through both Winch/Cranelift and the "weird cases" will be much more defined.
saulecabrera commented on issue #6610:
I finally got a chance to get back to this! I decided to try the approach of ignoring everything except for the
winch
misc test suite, I believe this will require the least amount of changes once we decide to enable everything by default in the future.This should be ready for a review @alexcrichton; one thing to note is that I didn't add the
if
test suite yet, because I found one bug that I'm currently trying to figure out, once I do, I'll push the fix along with the test suite.
Last updated: Dec 23 2024 at 12:05 UTC