Merge*Type on their respective class (Type, Memory, ...)

[jbourassa]

This is a proposal to change the API to address @sandstrom's feedback in #13. The general ideal is to remove the *Type (FuncType, MemoryType, GlobalType, TableType), collapsing their API surface on the Func / Memory / etc.

This PR isn't polished (I need to review the docs and proof-read everything), but putting it out there to collect feedback on the idea.

API changes

Func

Params and results Array are now on Func.new. FuncType#params and FuncType#results are moved on Func.

# Before
func_type = FuncType.new([:i32], [:i32])
func = Func.new(store, func_type) { ... }

func_type.params # => [:i32]
func_type.results # => [:i32]

# After
func = Func.new(store, [:i32], [:i32]) { ... }

func.params # => [:i32]
func.results # => [:i32]

Memory

Min and max size are now on Memory.new as kwargs (min_size is required, max_size is optional). MemoryType#minimum and #maximum are now Memory#min_size and #max_size.

I chose min and max as it's shorter and is commonly used in Ruby (Array#min, #max, #minmax).

# Before
mem_type = `MemoryType.new(1, 2)`
mem = Memory.new(store, mem_type)

mem_type.minimum # => 1
mem_type.maximum # => 2

# After
mem = Memory.new(store, min_size: 1, max_size: 2)
mem.min_size # => 1
mem.max_size # => 2

Global

Moved the .const and .var constructor from GlobalType to Global. Moved GlobalType#content to Global#type -- unifying with Table (see below). Unlike Rust, type isn't a reserved word in Ruby, and felt more appropriate than content.

# Before
global_type = GlobalType.var(:i32)
global = Global.new(store, global_type, 1)

global_type.var? # => true
global_type.const? # => false
global_type.element # => :i32

# After
global = Global.var(store, :i32, 1)
global.var? # => true
global.const? # => false
global.type # => :i32

Table

Moved type as as positional argument and min_size and max_size as kwarg on Table.new. Moved all TableType methods to Table, using similar naming as Memory (min_size, max_size) and Global (type).

# Before
table_type = TableType.new(:funcref, 1, 2)
table = Table.new(store, table_type, nil) # nil: default value

table_type.element # => :funcref
table_type.minimum # => 1
table_type.maximum # => 2

# After
table = Table.new(store, :funcref, nil, min_size: 1, max_size: 2)
table.type # => :funcref
table.min_size # => 1
table.max_size # => 2

[saulecabrera]

I don't have objections on the approach. My main concern (before going over the change) was the overhead that this might introduce, but I'm happy to see that this change is pretty much re-using the validation machinery that was there before (.e.g FuncType). Also ruby-wise, this seems more natural.

[sandstrom]

Overall I think this is great! Makes the API much easier to work with. 🎉

With the caveat that I don't know WASM or the Rust Wasmtime library very well, here is some feedback on the API:

Func

• I'd use kwargs, easier to extend func constructor with more stuff later, and
• with kwgargs it'll be more explicit (easier to read/understand the code)

# ALT 1
func = Func.new(store, params: [:i32], results: [:i32]) { ... }

# ALT 2
func = Func.new(store, param_types: [:i32], result_types: [:i32]) { ... }

Global

My understanding is that both a var and const would be instances of Global. In other words, that:

Global.var(store, :i32, 1).is_a?(Global) #=> true
Global.const(store, :i32, 1).is_a?(Global) #=> true

Building instances with something other than .new is something I'd avoid, if possible.

For me, any of these three alternatives would seem more intuitive:

# ALT 1
global = Global.new(store, mutability: :var, type: :i32, value: 1)
global.var? # => true
global.const? # => false
global.type # => :i32

global.mutability # => :var

# ALT 2
global = Global.new(store, mutable: true, type: :i32, value: 1)
global.var? # => true
global.const? # => false
global.type # => :i32

global.mutable? # => true

# ALT 3
global = Global.new(store, :var, :i32, 1)
global.var? # => true
global.const? # => false
global.type # => :i32

Which one is ideal depends a bit on whether there are more types of mutability that is anticipated in the future.

In alt 1 and 2, one could make the mutability kwarg optional, for added convenience at the cost of less explicit code. Also, in both 1 and 2, one could potentially drop the var? and const? getters.

One could also mix the three method signature alternatives above, for example Global.new(store, :i32, 1, mutable: true) (this would work well if mutable was an optional kwarg).

Table

:func_ref would be easier to read in my view.

It seem to be FuncRef in Rust, which I'd say would be the equivalent of func_ref in Ruby (snake_case is the standard style in Ruby).

[jbourassa]

@sandstrom Thanks for the feebdback!

Func

• I'd use kwargs, easier to extend func constructor with more stuff later, and
• with kwgargs it'll be more explicit (easier to read/understand the code)

I agree kwargs come with an added readability bonus. In a way, that'd match what WAT does ((func (param i32) (result f64) ...)). On the other hand, they make everything a little more verbose 🤔 . I'll sleep on this one.

---

My understanding is that both a var and const would be instances of Global.

That's correct. My rationale for that was to avoid the bool flag. I'm tempted to leave this one as is, particularly given it's easy to add .new in a non-breaking way later.

:func_ref would be easier to read in my view.

For that we'll stick with :funcref / :externref given that's what's in the spec and in the WebAssembly Text Format (WAT).

[ianks]

As far as design goes, I’m ok with a less vinegary interface given:

1. it doesn’t hurt performance
2. it doesn’t make it harder to auto-generate bindings in the future

There definitely needs to be a more Ruby-ish interface all of this stuff, but the primary goal of this gem is to have a robust foundational layer for wasmtime in Ruby.

The ergonomic interfaces will come later in other gems and will be much less of a headache to work with (👋 wit-bindgen-rb, wasmtime-ffi, and other awesome gems the community will write!).

That all being said, I don’t think this PR goes against points 1 or 2 so I’m conceptual 👍🏻 on it. Just wanted to set clear expectations

[jbourassa]

Thanks for chiming in Ian.

but the primary goal of this gem is to have a robust foundational layer for wasmtime in Ruby.

Couldn't agree more. That principle has been guiding the design of the gem. I think it's time I document the project goals in the README.