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

Array types in function arguments #43

Closed
2 tasks done
ballercat opened this issue Dec 25, 2017 · 7 comments
Closed
2 tasks done

Array types in function arguments #43

ballercat opened this issue Dec 25, 2017 · 7 comments
Milestone

Comments

@ballercat
Copy link
Owner

ballercat commented Dec 25, 2017

Overview

Array types should be acceptable function arguments. Currently, we do not have parser support for them.

Expected

A function can be declared with an array type as argument.

Actual

SyntaxError: 
function testa(arr: i32[]): void {
                       ^ Unexpected token Punctuator
Expected: "Identifier"
  at testa (unknown.walt:7:23)
walt.js:3238:12

Acceptance Criteria

  • Tests for the above case
  • Tests pass
@ballercat ballercat added this to the Alpha milestone Dec 25, 2017
@baransu
Copy link
Contributor

baransu commented Dec 26, 2017

Personaly I'm not a fan of i32[] syntax but it makes more sense right now because while we don't have generic types. After that it makes more sense to be more consistent and use Array<i32> syntax. It's more code but it's also valid and we're not creating special case for the array type.

@iddan
Copy link

iddan commented Dec 26, 2017

Isn't it a goal to support most of the Flow syntax?

@baransu
Copy link
Contributor

baransu commented Dec 26, 2017

Flow supports both notatations and i32[] is a shorthand for Array<i32>.

https://flow.org/en/docs/types/arrays/

@iddan
Copy link

iddan commented Dec 26, 2017

Yeah, so I assumed walt would do the same

@ballercat
Copy link
Owner Author

Isn't it a goal to support most of the Flow syntax?

I want to clear something up. The goal is to use flow to accomplish a specific goal of using JavaScript syntax to write WebAssembly. Supporting most of flow would mean supporting most of JavaScript, which is unlikely.

The Array<i32> syntax does make sense if you expect flowtype to be re-implemented here. But re-implementing flow is not really a goal, simply a means to an end. I also wonder if there is a better argument made for the additional syntax other than I prefer it over the other because to me it's not clear why either superior to the other.

Having an i32[] syntax is good because it's simple to implement and simple to understand and it's rather stable at this point. I'd like to see this project move past the alpha stage and have some real-world examples out there before exploring additional syntax. Like @baransu said having an Array<> style syntax implies that there might be some generics-like stuff around, which there isn't.

@baransu
Copy link
Contributor

baransu commented Dec 26, 2017

Maybe we can add flow as a part of the compiler? As you said reimplementing it isn't the goal but maybe we can leverage work that already been done. If we want to follow JavaScript and Flow syntax we can add flow as the first step of the validation. After that we will have the quarantee that we have correct typing so compiler can focus on more important things.

@iddan
Copy link

iddan commented Dec 26, 2017

Can Walt utilise the Flow AST?

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

No branches or pull requests

3 participants