Allow parsing of blocks with a type reference #4236
Conversation
pmatos
force-pushed
the
block-type-work
branch
2 times, most recently
from
0675ab1 to
8b6dbc6
Compare
Blocks are not allowed to have input parameters. Adds new test `blocks.wast`. Currently only blocks of the form are supported: ``` (block $label (result ...) ...) ``` This patch enables this type of block declaration: ``` (type $ty (func (param) (result ...))) (block $label (type $ty) ...) ```
pmatos
force-pushed
the
block-type-work
branch
from
8b6dbc6 to
22dfe04
Compare
|
This is still breaking the validator. There's some strange case where apparently the parser is for: generating a |
| // FIX: If we don't pass a second argument, this will call Type::Type(Rtt) | ||
| // instead of Type::Type(Ref, NonNullability), although at this point | ||
| // I am not sure the NonNullable here is the right thing to do. | ||
| return Type(parseTypeRef(typeref), NonNullable); |
There was a problem hiding this comment.
This is definitely strange, if I don't pass the NonNullable, I would expect this to create a Ref but it creates a Rtt simply because the constructor is the one that expects a single argument.
There was a problem hiding this comment.
I think the implicit conversions cause that, and they can be a little confusing, but they do make sense. An RTT can be constructed from a heap type, but nothing else - a reference must be either nullable or not, so that is not optional in its constructor.
But maybe the issue here is that parseTypeRef is not the right thing to call. That returns a heap type, but here we actually want a type. That is, it needs to parse something that is more than just a heap type, including whether it is null or not.
There was a problem hiding this comment.
Looks like the only existing caller to parseTypeRef is in type declarations, where indeed heap types are declared. So perhaps it should be renamed to parseHeapType?
| @@ -575,6 +575,11 @@ void FunctionValidator::validateNormalBlockElements(Block* curr) { | |||
| } | |||
| if (curr->list.size() > 0) { | |||
| auto backType = curr->list.back()->type; | |||
| if (curr->type.isRef()) { | |||
There was a problem hiding this comment.
I added this just to check how far we could go but not very far because there's some validation issue then later on the visitFunction. I think at this point, we need to check that the referenced type has no inputs, but I can't find a way to access the referenced TypeInfo (I think it's where it's stored). Any hints here?
| (module | ||
| (type $ty (func (param) (result i32))) | ||
|
|
||
| ;; (func $withblock-result (param) (result i32) |
There was a problem hiding this comment.
This is ok and passing, so to simplify my life I just commented it.
|
Probably a new test in |
Blocks are not allowed to have input parameters.
Currently only blocks of the form are supported:
This patch enables this type of block declaration: