Skip to content

Implement preliminary creation of Wasm GC types #2537

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

Merged
merged 11 commits into from
Oct 21, 2022
Merged

Implement preliminary creation of Wasm GC types #2537

merged 11 commits into from
Oct 21, 2022

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Oct 16, 2022

Adds logic to incrementally create Wasm GC types with Binaryen's type builder. Is necessary because Binaryen is switching to utilize GC types internally to have all the information, even when the GC feature is disabled, then emitting reference types. Required by #2531 so we can create ref.func expressions with concrete types (cc @MaxGraey).

Still some TODOs:

  • There is no impl for GC array members yet. An Array<T> for example would wrap a GC array, while an ArrayBuffer would be represented by one. However, since the respective code and stdlib changes have not yet been made, yet would go in tandem with the types, this is left out for now. My expectation is that we'd add a source-level (lower case) array<T> type that is directly bound to the respective Wasm GC type for stdlib to use, perhaps somewhat like:
    class Array<T> {
      buffer: array<T>;
      ...
    }
    class StaticArray<T> extends array<T> {
      ...
    }
    class ArrayBuffer extends array<u8> {
      ...
    }
  • GC type creation has been tested against the full test suite, being able to represent all the types we have as GC types (with the above limitation), except for one test: duplicate-fields. This test includes a class structure akin to the following:
    class Foo {}
    class Bar extends Foo {}
    class A {
      foo: Foo;
    }
    class B extends A {
      foo: Bar;
    }
    where there is a duplicate field foo with a subtyping relationship. This is sound afaict, but appears to be not supported by Wasm GC, unless both foos would be immutable fields, which they aren't in our case. Looks like we'd have to revert support for specializing duplicate fields like this in order to use Wasm GC (see below).
  • Building GC types on the side has side effects (on the text format), in that in order to create the types, a type builder must be created, which requires setting a type system, and then leads to Wasm GC text format with func_subtype and whatnot, even though the GC feature is disabled. See: Regression when generating ref.func? WebAssembly/binaryen#5136 (comment)
  • Regarding Update Binaryen #2531, what this allows us to do is to ensureType(theFunctionSignature) for use with Module#ref_func to make it work again, even if the GC type isn't otherwise used and Binaryen would output funcref iiuc.
  • I've read the contributing guidelines
  • I've added my name and email to the NOTICE file

Copy link
Member

@MaxGraey MaxGraey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dcodeIO
Copy link
Member Author

dcodeIO commented Oct 16, 2022

Reading the GC explainer wrt. the failing duplicate-fields test, the rules in play are

Structure types support width and depth subtyping

  • struct <fieldtype1>* <fieldtype1'>* <: struct <fieldtype2>*
    • iff (<fieldtype1> <: <fieldtype2>)*

Field types are covariant if they are immutable, invariant otherwise

  • const <valtype1> <: const <valtype2>
    • iff <valtype1> <: <valtype2>
  • var <valtype> <: var <valtype>
  • Note: mutable fields are not subtypes of immutable ones, so const really means constant, not read-only

i.e. mutable fields are invariant. I guess there is a reason for this limitation, though I don't currently see it. I guess we could work around this by making the duplicate field on B in the above example a property with accessors that do casts from/to Bar and store to an inherited foo: Foo, rendering the field invariant behind additional casting cost. Alternatively, the casts could be inserted inline.

Edit: Reason is, of course, that

class Animal {
  parent: Animal;
}
class Cat extends Animal {
  parent: Cat;
}
class Dog extends Animal {
  parent: Dog;
}

let animal: Animal = new Cat();
animal.parent = new Dog();

is allowed by covariance but unsound, so we should revert to make this actually type safe. Was introduced in #2158.

@MaxGraey MaxGraey mentioned this pull request Oct 20, 2022
2 tasks
@dcodeIO dcodeIO merged commit 78b2d1a into main Oct 21, 2022
@HerrCai0907 HerrCai0907 deleted the gc-types branch October 17, 2023 08:59
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.

2 participants