Reference types changes to remove subtyping #1407
Conversation
|
This PR can't land as-is, since it fails all the spec tests (that need to be updated) |
| @@ -157,8 +157,7 @@ The `reference-types` proposal adds three more valid types: | |||
|
|
|||
| | type | JSON format | | |||
| | - | - | | |||
| | "nullref" | `{"type": "nullref", "value": ""}` | | |||
| | "hostref" | `{"type": "hostref", "value": <string>}` | | |||
| | "externref" | `{"type": "externref", "value": <string>}` | | |||
| | "funcref" | `{"type": "funcref", "value": <string>}` | | |||
There was a problem hiding this comment.
I notice there that we are missing exnref .. which is a little annoyingly close in name to externref
| return true; | ||
| } | ||
| return false; | ||
| return expected == actual; |
There was a problem hiding this comment.
Maybe a comment here as to why this a function at all still? Something like "we used to have subtyping and we expect we may again in the future"?
src/interp/interp-util.cc
Outdated
| case Type::Funcref: | ||
| return StringPrintf("funcref:%" PRIzd, tv.value.Get<Ref>().index); | ||
|
|
||
| case Type::Externref: |
There was a problem hiding this comment.
What do you think about camel-casing these (as a followup). I find ExternRef a little easier to read than Externref
| @@ -15,8 +17,7 @@ struct TokenInfo { | |||
| }; | |||
| }; | |||
| %% | |||
| anyref, Type::Anyref | |||
| array, TokenType::Array | |||
| array, Type::Array, TokenType::Array | |||
There was a problem hiding this comment.
Why did this and other seemingly unrelated lines in this file change?
There was a problem hiding this comment.
Maybe another possible split out PR?
| Result BinaryReaderLogging::name(Index value, Type type) { \ | ||
| LOGF(#name "(index: %" PRIindex ", type: %s)\n", value, type.GetName()); \ | ||
| return reader_->name(value, type); \ | ||
| } |
There was a problem hiding this comment.
This part could be split out if you feel like landing sooner.
|
Is there a PR in the reftypes proposal you can link to here? |
|
Here's the link to the reference-types PR: WebAssembly/reference-types#87. I'll add it to the commit too. |
Main changes: * Rename `anyref` -> `externref` * Remove `nullref` * Rename `hostref` -> `externref` * `ref.null` and `ref.is_null` now have "ref kind" parameter * Add ref kind keywords: `func`, `extern`, `exn`
Main changes:
anyref->externrefnullrefhostref->externrefref.nullandref.is_nullnow have "ref kind" parameterfunc,extern,exn