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

Move signatures into module types table #133

Merged
merged 5 commits into from
Oct 14, 2015
Merged

Conversation

lukewagner
Copy link
Member

In preparation for updating function pointers to match design/#392, a module-level table of signatures is needed which is indexed by call_indirect and func definitions (replacing the signature stored inside func with an index into the table).

This PR allows explicit declaration of the type table (currently only containing function types, but one could imagine other compound types in the future) via (types (func $T1 (param f32) (result i64)) ...) (where T1 desugars to index, like other names). The existing syntax for defining functions still works and desugars to adding the signature to the table (if it's not already there) and using the resulting index. However, types can be explicitly referenced via (func (type $T1) ...). Both can be used (so that parameters can be named), in which case the signatures must match.

| Some _ -> Error.error at "more than one return type"
| None -> {f with result = Some $3} }
{ {(fst $5) with ins = $3 @ (fst $5).ins},
fun c -> anon_locals c $3; (snd $5) c }
Copy link
Member Author

Choose a reason for hiding this comment

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

Just a note to reviewers: so the return value of func_fields is now a pair, instead of just the function. This was necessary to extract the type during the first pass (when the global scope is built) instead of the second pass (where function bodies are generated). Alternatively, I played a bit with moving the param/result fields out of func_fields and into another production that came before func_fields in func, but I ran into shift/reduce and reduce/reduce conflicts that seemed to require changing the text format to better isolate the signature.

@rossberg
Copy link
Member

Mostly looks good, but I have a couple of bike-shedding comments.

  • It seems unnecessary to force grouping type definitions into a "section" -- we don't require that for other things either. Just have module fields of the form (type ...). That's both more flexible and simplifies the grammar some.
  • Don't nest the bound type name inside the type expression -- that's such a C-ism ;). Can we write type defs as (type $t (func (param ...) (result ...)))? Besides being more logical, such a syntax also scales better to other forms of type definition we might want to add later.

Also, README update is missing. :)

@lukewagner
Copy link
Member Author

@rossberg-chromium My motivation wasn't so much C as matching the structure of (func ...) definitions which was arbitrary. Anyhow, both changes you suggested sound good. All comments should be addressed with the most recent push.

Note: we could greatly simplify func and probably reuse type_def if we switched the structure of func definitions to: (func $foo (type (param ...) (result ...)) (local ...) ...). This changes the text format and makes it more verbose in the common case, though. wdyt?

@rossberg
Copy link
Member

That would require changing the syntax of function types, though, so that you can still bind parameter names, doesn't it?

@rossberg
Copy link
Member

Anyway, LGTM

@lukewagner
Copy link
Member Author

That would require changing the syntax of function types, though, so that you can still bind variable
names, doesn't it?

Oh, hah, right; I realized this when I was trying it out earlier but then forgot.

lukewagner added a commit that referenced this pull request Oct 14, 2015
Move signatures into module types table
@lukewagner lukewagner merged commit c1c1c9a into master Oct 14, 2015
@lukewagner lukewagner deleted the factor-signature branch October 14, 2015 17:20
lukewagner added a commit that referenced this pull request Oct 14, 2015
alexcrichton pushed a commit to alexcrichton/spec that referenced this pull request Nov 18, 2019
[test] Integrate v128.andnot tests from WAVM
Connicpu pushed a commit to Connicpu/wasm-spec that referenced this pull request May 11, 2020
rossberg added a commit that referenced this pull request Feb 11, 2021
dhil pushed a commit to dhil/webassembly-spec that referenced this pull request Mar 2, 2023
This updates the explainer text according to the new spec we agreed in
the 09-15-2020 CG meeting and discussions afterwards.

The following are modifications and clarifications we made after the
09-15-2020 CG meeting, and the relevant issue posts, if any:
https://github.com/WebAssembly/meetings/blob/master/main/2020/CG-09-15.md

- `catch_br` wasm renamed to `delegate` (WebAssembly#133)
- `rethrow` gains an immediate argument (WebAssembly#126)
- Removed dependences on the reference types proposal and the multivalue
  proposal. The multivalue proposal was previously listed as dependent
  because 1. `try` is basically a `block`, so it can have multivalue
  input/output 2. `br_on_exn` can extract multiple values from a
  `block`. We don't have `br_on_exn` anymore, and I'm not sure 1 is a
  strong enough reason to make it a dependence.
- Mention `rethrow` cannot rethrow exceptions caught by `unwind` (WebAssembly#142
  and WebAssembly#137)
- Mention some runtimes, especially web VMs, can attach stack traces to
  the exception object, implying stack traces are not required for all
  VMs
- Update label/validation rules for `delegate` and `rethrow` (WebAssembly#146)
- Finalize opcodes for `delegate` (0x18) and `catch_all` (0x19) (WebAssembly#145
  and WebAssembly#147)

I believe this resolves many previous issue threads, so I'll close them.
Please reopen them if you think there are things left for discussions in
those issues.

Resolves WebAssembly#113, resolves WebAssembly#126, resolves WebAssembly#127, resolves WebAssembly#128, resolves
WebAssembly#130, resolves WebAssembly#142, resolves WebAssembly#145, resolves WebAssembly#146, resolves WebAssembly#147.
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