Skip to content
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

Update binaryen + improve codegen of instantiation #2302

Merged
merged 8 commits into from
May 29, 2022

Conversation

MaxGraey
Copy link
Member

According to WebAssembly/binaryen#4633 externref is alias of anyref now.

  • I've read the contributing guidelines
  • I've added my name and email to the NOTICE file

@MaxGraey
Copy link
Member Author

@dcodeIO But it seems this changes in Binaryen produce some issues in features/reference-types test (instantiate error (debug)). Any ideas?

@MaxGraey MaxGraey changed the title Update binaryen [WIP] Update binaryen May 28, 2022
@dcodeIO
Copy link
Member

dcodeIO commented May 28, 2022

The error reads:

CompileError: WebAssembly.instantiate():
  Compiling function #8:"start:features/reference-types" failed:
    global.set[0] expected type externref, found global.get of type funcref @+929

What might have happened is that previously there was externref, which is disjoint from funcref, and their common supertype anyref, which hence can be assigned a funcref,

       anyref
       /   \
externref  funcref

but now there's just anyref = externref

  anyref  funcref

so a test case along the lines of

var ref: anyref = someFuncref

fails. Unfortunate that the design is going to be that an anyref cannot be assigned any ref, respectively that one can't pass a JS function into Wasm as an anyref (and call it without glue). Poor AS.

Relevant spec discussions:

@MaxGraey MaxGraey changed the title [WIP] Update binaryen Update binaryen May 28, 2022
@MaxGraey MaxGraey requested a review from dcodeIO May 28, 2022 16:17
@MaxGraey MaxGraey changed the title Update binaryen Update binaryen + improve codegen of instantiation May 28, 2022
@dcodeIO
Copy link
Member

dcodeIO commented May 28, 2022

Would you like to look into removing externref (respectively what was anyref) in the compiler as well (so assigning funcref for example won't fail), or would you prefer to keep this to a follow-up PR? I guess these can be split since the PR doesn't change the status quo as-is.

@MaxGraey
Copy link
Member Author

Yes, it is better to do it in a separate PR as it will be breaking changes

@dcodeIO dcodeIO merged commit 324d41c into AssemblyScript:main May 29, 2022
@MaxGraey MaxGraey deleted the upd-binaryen branch May 29, 2022 05:12
@tlively
Copy link

tlively commented Jun 1, 2022

Unfortunate that the design is going to be that an anyref cannot be assigned any ref, respectively that one can't pass a JS function into Wasm as an anyref (and call it without glue).

I don't understand what you mean here. anyref is still the top reference type so it is able to hold funcref values and any other reference type value (at least unless we decide otherwise on WebAssembly/gc#293). Calling a JS function passed in as an anyref would require downcasting it to funcref so it can be called, but that should work.

@dcodeIO
Copy link
Member

dcodeIO commented Jun 1, 2022

Hey @tlively :) From discussion so far and reftypes history, I'd be surprised if funcref <: anyref remained, also since the major stakeholders appear to favor funcref </: anyref. If so, that'd leave us with an anyref that cannot be assigned any ref including what it entails. Is my expectation off?

@tlively
Copy link

tlively commented Jun 1, 2022

If the result of the discussion is funcref </: anyref, then yes, that would be correct. I wouldn't be so sure of the result quite yet, though! If I had to guess, I'd say there's a 70% chance we keep funcref <: anyref given the info I have right now. Can you tell me a bit about how AS depends on or uses funcref <: anyref today?

@dcodeIO
Copy link
Member

dcodeIO commented Jun 1, 2022

That's good to hear! Apart from verifying assumptions in feature tests as of this issue, AS does not in practice depend on funcref <: anyref today. For a JS-inspired language with plans for seamless interop, a catch-all reference type as of anyref seems desirable, though, so for example any JS value can be piped through a Wasm export to a JS import that accepts any JS value. Being able to inspect, in just Wasm, that the anyref is a callable funcref, may similarly be handy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants