WebAssembly / gc

Branch of the spec repo scoped to discussion of GC integration in WebAssembly
https://webassembly.github.io/gc/
Other
982 stars 70 forks source link

Type of active element segments #538

Closed manoskouk closed 4 months ago

manoskouk commented 4 months ago

According to the spec, active element segments without an explicit type have type (ref null func) rather than their respective table's type. Is this intended? If so, we don't seem to have a spec test that covers it.

manoskouk commented 4 months ago

I am also now noticing that kind 0 and 4 element segments are defined to have type (ref null func) over (ref func). Is this intended?

rossberg commented 4 months ago

Both properties are intended. The first is to maintain phase separation: in general, the table type is not known during decoding but only during validation, which is too late to use it for parsing disambiguation. The second is for backwards compatibility, because active segments already existed in Wasm 1.0/2.0, i.e., predate non-nullable reference types.

There should be tests for kind 0, since that's the Wasm 1.0 case. Not sure about kind 4, though I thought we have some tests for each case (can't check right now). If not, a PR is of course welcome. :)

manoskouk commented 4 months ago

Then shouldn't this test fail? It declares a table of type (ref func) but the element segment referring to it is typed (ref null func).

rossberg commented 4 months ago

Ah, sorry, my mistake, I misremembered. It seems like we indeed refined active segments to be non-nullable (which is conservative). I'll fix the spec.

rossberg commented 4 months ago

See WebAssembly/function-references#113

manoskouk commented 4 months ago

I think we overcompensated here. I think case 4 should still allow for null references.

rossberg commented 4 months ago

Hm, maybe. But that would be an one-off exception from all the other places. Because in fact, I just noticed that there is another place that should have been updated: elemkind should also produce (ref func). That's what the reference interpreter currently does, consistently using (ref func) everywhere.

Either way, I agree that there should be tests. Looking into that just now I realise that the difficulty here is that there actually is no way to directly express some of these cases in the text format, so we'll have to write them in binary.

manoskouk commented 4 months ago

But that would not be backwards compatible, at least according to current spec. In current spec, we do not have (ref func), and null references are allowed in element segments.

rossberg commented 4 months ago

I suppose you are right. Will fix.

rossberg commented 4 months ago

Please see WebAssembly/function-references#114, which also adds tests.