[test] Add more tests for cont.new#147
Conversation
| let run_action act : Value.t list = | ||
| match act.it with | ||
| | Invoke (x_opt, name, vs) -> | ||
| | (Invoke (x_opt, name, vs): Wasm.Script.action') -> |
There was a problem hiding this comment.
The complaint about the module recursion must be because this refers back to module Wasm. Just drop the prefix.
| type 'ctxt t = 'ctxt cont | ||
| and 'ctxt cont = int32 * 'ctxt (* TODO: represent type properly *) |
There was a problem hiding this comment.
Nit: these don't need to be recursive, just put cont first.
|
|
||
| let alloc (GlobalT (_mut, t) as ty) v = | ||
| assert Free.((val_type t).types = Set.empty); | ||
| if not (Match.match_val_type [] (type_of_value v) t) then raise Type; |
There was a problem hiding this comment.
This was causing allocation of globals with defined continuation types to fail because type_of_value rerurns the top cont type for continuation values.
| type 'a stack = 'a list | ||
|
|
||
| type frame = | ||
| { | ||
| inst : module_inst; | ||
| locals : value option ref list; | ||
| } | ||
|
|
||
| type code = value stack * admin_instr list | ||
|
|
||
| and admin_instr = admin_instr' phrase | ||
| and admin_instr' = | ||
| | Plain of instr' | ||
| | Refer of ref_ | ||
| | Invoke of func_inst | ||
| | Breaking of int32 * value stack | ||
| | Returning of value stack | ||
| | ReturningInvoke of value stack * func_inst | ||
| | Throwing of Tag.t * value stack | ||
| | Trapping of string | ||
| | Label of int * instr list * code | ||
| | Frame of int * frame * code | ||
| | Handler of int * catch list * code | ||
| | Prompt of handle_table * code | ||
| | Suspending of tag_inst * value stack * (int32 * ref_) option * ctxt | ||
|
|
||
| and ctxt = code -> code | ||
| and handle_table = (tag_inst * idx) list * tag_inst list |
There was a problem hiding this comment.
Is there a reason you want to expose all this? If you need type ctxt in the interface to define cont below, it suffices to export it abstractly (justtype ctxt), and then all the other types don't need to be here.
There was a problem hiding this comment.
No reason, this is just copied out of the .ml file and I didn't know it could be shortened. Will give it a shot.
tlively
changed the title
Also add support for `ref.cont` return patterns in assertions.
|
@rossberg I've significantly simplified the changes here. PTAL! |
rossberg
left a comment
rossberg
left a comment
There was a problem hiding this comment.
Looks mostly good, but you'll also need to extend the is_js_reftype predicate in script/js.ml, otherwise it will assume that it can im/export conts to/from JS instead of creating wrapper modules (for assertions over cont refs).
| type ctxt | ||
| type handle_table | ||
|
|
||
| type cont = int32 * ctxt (* TODO: represent type properly *) |
There was a problem hiding this comment.
Since these types aren't used externally, I believe we can simplify even further:
| type ctxt | |
| type handle_table | |
| type cont = int32 * ctxt (* TODO: represent type properly *) | |
| type cont |
What's the simplest way to run the JS tests that would expose this bug? |
See the commented-out CI interpreter workflow (presuming Node already has it, otherwise try the same with d8): |
2 participants
Also add support for
ref.contreturn patterns in assertions.