[WIP] Use Type instead of HeapType for functions #7971
Conversation
|
As discussed, I'll take a look at finishing this. |
|
@tlively this builds and has main merged in, but it has test errors. I actually am not sure how you were planning on fixing these. E.g. the We could give all function types inexact types for now..? Or just do the larger change as part of this one? |
|
That wasm-merge error should go away if we keep checking subtyping only on the heap types for now. |
|
I worry that might be a bit of a tightrope to walk here... may be better to fix it all in one PR. But I added that now. |
|
@tlively this should be good to go |
src/passes/StringLifting.cpp
Outdated
| auto type = func->type; | ||
| if (func->base == "fromCharCodeArray") { | ||
| if (type != Signature({array16, i32, i32}, refExtern)) { | ||
| if (type.with(Inexact).getHeapType() != |
There was a problem hiding this comment.
Since we treat exactness as being a property of reference types rather than heap types, the .with(Inexact) here is not doing anything. Is it there so we remember to remove it along with the .getHeapType() in the future?
There was a problem hiding this comment.
Ah, I must have done this in two parts, and missed that the latter made the former redundant. Fixed.
src/wasm/wasm-validator.cpp
Outdated
| // TODO: Compare the full Type | ||
| shouldBeEqual(curr->type.getHeapType(), | ||
| func->type.getHeapType(), |
There was a problem hiding this comment.
This one
| // TODO: Compare the full Type | |
| shouldBeEqual(curr->type.getHeapType(), | |
| func->type.getHeapType(), | |
| // TODO: Compare the full Type | |
| shouldBeEqual(curr->type, | |
| func->type, |
If this change works as intended, both sides will always be non-nullable and exact here. Once we start making some imports inexact, this will help make sure everything stays internally consistent.
There was a problem hiding this comment.
Oh, not sure why I thought this one must be deferred. Good point, it was a bug, in the fuzzer. Fixed.
Co-authored-by: Thomas Lively <tlively@google.com>
Co-authored-by: Thomas Lively <tlively@google.com>
Co-authored-by: Thomas Lively <tlively@google.com>
Co-authored-by: Thomas Lively <tlively@google.com>
Co-authored-by: Thomas Lively <tlively@google.com>
|
I cannot approve this in the GitHub UI since I'm the one who originally uploaded it, but LGTM. Thanks for taking this over! |
|
A few thousand fuzzer iterations without errors, landing. |
This will let us enforce that the type of a
ref.funcis equal to the type of the referenced function, even once we introduce inexact function imports.