-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Error linking to function that takes a GC array ref. #9848
Comments
Thanks for filing an issue. I won't have time to look into this for a while, unfortunately, but I appreciate getting it on the record for posterity. |
Subscribe to Label Actioncc @fitzgen
This issue or pull request has been labeled: "wasmtime:ref-types"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
FWIW, this is probably related to the |
Hm so I converted this into a test in #[test]
fn instantiate_with_func_that_takes_array() -> Result<()> {
let mut store = gc_store()?;
let module = Module::new(
store.engine(),
r#"
(module
(type $String (;3;) (array i8))
(type $debug.type (;4;) (func (param (ref $String))))
(import "host" "debug" (func $debug (;0;) (type $debug.type)))
)
"#,
)?;
let mut linker = wasmtime::Linker::new(store.engine());
linker.func_wrap("host", "debug", |_: Caller<'_, ()>, _: Rooted<ArrayRef>| {})?;
let _instance = linker.instantiate(&mut store, &module)?;
Ok(())
} And I am indeed reproducing the error:
However, I'm not convinced that this is actually incorrect behavior. The type of the wrapped function is If we make #[test]
fn instantiate_with_func_that_takes_array() -> Result<()> {
let mut store = gc_store()?;
let module = Module::new(
store.engine(),
r#"
(module
(type $String (;3;) (array i8))
(type $debug.type (;4;) (sub (func (param (ref $String)))))
(import "host" "debug" (func $debug (;0;) (type $debug.type)))
)
"#,
)?;
let mut linker = wasmtime::Linker::new(store.engine());
let string_ty = ArrayType::new(
store.engine(),
FieldType::new(Mutability::Const, StorageType::I8),
);
let sup_ty = FuncType::with_finality_and_supertype(
store.engine(),
Finality::NonFinal,
None,
[ValType::Ref(RefType::new(
false,
HeapType::ConcreteArray(string_ty),
))],
None,
)?;
let sub_ty = FuncType::with_finality_and_supertype(
store.engine(),
Finality::NonFinal,
Some(&sup_ty),
[ValType::Ref(RefType::new(false, HeapType::Array))],
None,
)?;
linker.func_new("host", "debug", sub_ty, |_caller, _args, _rets| Ok(()))?;
let _instance = linker.instantiate(&mut store, &module)?;
Ok(())
}
That is all technically correct, but I'm not sure if it is what we ultimately want. It seems very reasonable to want to write code like in the OP, even if it technically doesn't match subtyping rules (but would if the types involved were non- One extra twist is that I think we actually used to allow this kind of thing, but I think I removed it when getting the GC wast tests passing, since (IIRC) there were some |
I always forget the directionality of subtyping but I'm confused why this doesn't pass the subtyping check. If I ask for Otherwise though I'd agree that we want something |
The issue is twofold: First, that the wrapped function's type, when fully desugared, is (sub final (func (param (ref array)))) which does not declare itself to be a subtype of And second, that (type $debug.type (func (param (ref $String))) desugars to a (type $debug.type (sub final (func (param (ref $String))))) so no other function type can be a subtype of This all makes sense inside the world of pure Wasm, but from a host API and developer niceties point of view, it becomes pretty frustrating and leads to bad developer experience :-/ |
Ah I see, I was mistakenly thinking about the I thought we had something though which was testing that as long as the top type was the same then the host-level type-check passed? And then we optionally included a runtime type-check if the actual type was different from the top type? I'm probably misremembering here... Another option perhaps is to do something different for wasm gc than we do for normal scalars. Some sort of argument passed to I suppose put another way one thing we could do is to change In any case I'm just musing here... |
I'm getting this error:
The wasm looks like this:
And I'm registering the function like this:
Test Case
Steps to Reproduce
Create an Engine and Linker.
Bind a function which accepts an ArrayRef as a parameter.
Attempt to load the script.
Expected Results
Script should load without error (it loads if I delete the code that calls the host func).
Actual Results
Error when I attempt to instantiate the script.
Versions and Environment
Wasmtime version or commit:
{ version = "27.0.0", features = ["gc", "gc-drc"] }
Operating system: OS X
Architecture: M2
The text was updated successfully, but these errors were encountered: