Conversation
I'm winging it--trad coding, if you can believe it! |
rossberg
left a comment
There was a problem hiding this comment.
LGTM mostly, but why did you not order the arguments of MemoryT consistently with TableT and with the text format, i.e., put the page type last?
Done. |
| @@ -196,7 +196,7 @@ let check_globaltype (c : context) (gt : globaltype) at = | |||
| check_valtype c t at | |||
|
|
|||
| let check_memorytype (c : context) (mt : memorytype) at = | |||
There was a problem hiding this comment.
Don't you need to check here that pt is either 1 or 0x10000?
| require (I64.le_u min max) at | ||
| "size minimum must not be greater than maximum" | ||
|
|
||
| let check_pagesize (PageT ps) at = |
There was a problem hiding this comment.
Nit: check_pagetype, since that's what you named the thing.
|
AFAICT the missing parts now are just the parser, decoder, and encoder, and then it should actually be able to pass tests. The bounds-check machinery is already based on byte-sized memories and doesn't really concern itself with the pagesize, except in the places I already modified. |
This PR is just to get the ball rolling by adding a page type to a memory type.