feat: properly parse and resolve tuples#3018
Conversation
it's not ready yet
Co-authored-by: Max Graey <maxgraey@gmail.com>
Co-authored-by: Max Graey <maxgraey@gmail.com>
Co-authored-by: Max Graey <maxgraey@gmail.com>
Co-authored-by: Max Graey <maxgraey@gmail.com>
HerrCai0907
left a comment
There was a problem hiding this comment.
LGTM.
But I think tuple does not rely on multi-value. Here is my implement of tuple in AS. maybe you can ref it if you are interesting to implement the whole tuple support. wasm-ecosystem/warpo#269
| type.range.start = startPos; | ||
|
|
||
| // '[' ((Identifier ':')? Type (',' (Identifier ':')? Type)*)? ']' | ||
| } else if (token == Token.OpenBracket && this.options && this.options!.hasFeature(Feature.MultiValue)) { |
There was a problem hiding this comment.
Actually tuple doesn't and is impossible to rely on mutli-value.
declare function f(): [i32,i32];
function b() {
let a = f();
let b = a;
b[0] = 1;
assert(a[0] == 1);
}In this cases, multiple value cannot handle this case since it treat the whole tuple as a value instead of a heap object.
There was a problem hiding this comment.
@HerrCai0907, I'll add full support for tuples in the future. Tuples will need to exist in two forms:
- Tuple becomes
multi valuewhen returned from function - When used as a heap type (eg.
store<[i32, i32]>(),Array<[i32, i32]>), it becomes a heap type
Then, depending on usage, tuples will be lifted and lowered from multi-value to tuple
There was a problem hiding this comment.
We’ve been discussing this for a long time, and it’s clear that the only way to ensure that tuples are properly converted to multivalues is to place a readonly attribute before them. Otherwise, we’ll have to analyze the control flow for side effects which required custom pass based on binaryen infra.
This PR adds basic support for the Tuples. Roadmap wise, this, along with a few more PRs, will pave the way for the Multi Value Proposal to be properly implemented.
The goal of this PR is not to add full-fledged support for
Tuples. Instead, it supports the basic functionality for parsing and resolving them since optimalTupleimplementation relies on Multi Value support.The order of the PRs to implement are:
binaryenmulti-valueby defaultSpeaking of that, all the features here hide behind
--enable multi-valueSo far, this implements:
Parser
function foo(x: [i32, i32]) {}function foo(): [i32, i32] {}return [0, 1]type Foo = [i32, i32][x: f64, y: f64]Compiler
not implemented(for now)I've read the contributing guidelines
I've added my name and email to the NOTICE file