Collapse assert_return_canonical_nan and assert_return_arithmetic_nan to assert_return#142
Collapse assert_return_canonical_nan and assert_return_arithmetic_nan to assert_return#142abrown wants to merge 1 commit intoWebAssembly:masterfrom
Conversation
… to assert_return
| of_assertion' mods act "assert_return_canonical_nan" [] (Some assert_return_canonical_nan) | ||
| | AssertReturnArithmeticNaN act -> | ||
| of_assertion' mods act "assert_return_arithmetic_nan" [] (Some assert_return_arithmetic_nan) | ||
| (Some (assert_return lits)) (* TODO *) |
There was a problem hiding this comment.
Not exactly sure if I understand what this is for; is it generating JS code? If that is the case, I likely need to match and emit different code for each branch.
There was a problem hiding this comment.
Yes, this is for generating a JS version of the test.
| ) | ||
|
|
||
| | AssertReturn (act, vs) -> | ||
| | AssertReturn (act, vs, modifier) -> |
There was a problem hiding this comment.
I foresee vs going away and matching on modifier here
There was a problem hiding this comment.
There wouldn't be a modifier here. Instead, vs would change from a list of values to a list of "results", which would be a new type.
| | AssertReturnConstant of Ast.literal list | ||
| | AssertReturnArithmeticNan (* TODO f32x4 | f64x2 *) | ||
| | AssertReturnCanonicalNan (* TODO f32x4 | f64x2 *) | ||
| (* TODO (ref.any) | (ref.func) *) |
There was a problem hiding this comment.
I can't seem to find types for ref.any or ref.func?
There was a problem hiding this comment.
Those would be added the reference types proposal.
| Node ("assert_return_canonical_nan", [action act]) | ||
| | AssertReturnArithmeticNaN act -> | ||
| Node ("assert_return_arithmetic_nan", [action act]) | ||
| Node ("assert_return", [action act]) (* TODO :: List.map literal lits) *) |
There was a problem hiding this comment.
This will need to handle modifier (or whatever we want to call it) instead of a list of literals but, to make sure, this is a serializer of the AST?
| | const_list { AssertReturnConstant ($1) @@ at () } | ||
| | LPAR ARITHMETIC_NAN /* TODO f32x4 | f64x2 */ RPAR { AssertReturnArithmeticNan @@ at () } | ||
| | LPAR CANONICAL_NAN /* TODO f32x4 | f64x2 */ RPAR { AssertReturnCanonicalNan @@ at () } | ||
| /* TODO (ref.any) | (ref.func) */ |
There was a problem hiding this comment.
Should I be able to implement the branches for ref.any and ref.func?
rossberg
left a comment
There was a problem hiding this comment.
Thanks for starting on this, but I think you'll need to implement the basic change upstream on the main spec repo first. Otherwise it will be extremely difficult to sync with other proposals that need to introduce their own extensions to the result syntax, like the reference types proposal.
See also my latest comment on #137 for a refinement of my suggestion.
| of_assertion' mods act "assert_return_canonical_nan" [] (Some assert_return_canonical_nan) | ||
| | AssertReturnArithmeticNaN act -> | ||
| of_assertion' mods act "assert_return_arithmetic_nan" [] (Some assert_return_arithmetic_nan) | ||
| (Some (assert_return lits)) (* TODO *) |
There was a problem hiding this comment.
Yes, this is for generating a JS version of the test.
| | AssertReturnConstant of Ast.literal list | ||
| | AssertReturnArithmeticNan (* TODO f32x4 | f64x2 *) | ||
| | AssertReturnCanonicalNan (* TODO f32x4 | f64x2 *) | ||
| (* TODO (ref.any) | (ref.func) *) |
There was a problem hiding this comment.
Those would be added the reference types proposal.
| ) | ||
|
|
||
| | AssertReturn (act, vs) -> | ||
| | AssertReturn (act, vs, modifier) -> |
There was a problem hiding this comment.
There wouldn't be a modifier here. Instead, vs would change from a list of values to a list of "results", which would be a new type.
|
Okay, I implemented a base version of this refactoring for the MVP spec in WebAssembly/spec#1104. Let's land this ASAP and then go from there. |
An attempt to resolve #137; this is still a WIP but I will update as I go along. Could use some help with questions annotated as comments on the code.