syntax icon indicating copy to clipboard operation
syntax copied to clipboard

Feature: relax the constraint of `...` in list construction

Open bobzhang opened this issue 3 years ago • 3 comments

Currently, ... is only allowed in the tail position, list {1,2,... tail}, For example:

let l = a => list{1,2,...a} // works
let l = a => list{1,2,...a, 3,  ...a} // fails

The current situation is a bit weird, it is not a syntax error, but a type error

This has type: int Somewhere wanted: list<int>

This also seems to be a bug:

let l = (a,b) => list{1,2, ...b , 3,...a}

The inferred type for b is int

bobzhang avatar Oct 11 '22 00:10 bobzhang

This issue seems raised from here. https://github.com/rescript-lang/syntax/blob/c2b5750f5fa8b9a50875ec7d6ff96319171f46a7/src/res_core.ml#L3727

The current behavior is: if the last list element expression is a spread, the parser will not check the rest element whether contains a spread expression. So the list expression

list{...a, ...b ,...a}

would be parsed to OCaml

a :: b :: a

which raises a type error in subsequent type checking.

But

list{...a, ...b, c}

would raise a syntax error, because the last is not a spread.

My commit seems fix this.

Would someone help review my code?

butterunderflow avatar Oct 12 '22 16:10 butterunderflow

Would you create a pull request?

cristianoc avatar Oct 12 '22 18:10 cristianoc

@butterunderflow A proposal for fix: when ... only appears in the tail position, nothing changes, so no regressions would happen.

when multiple ... appears

 list{ a0 , ... b0 , ai, ... bi, }

==> Belt.List.concatMany ([ list{a0, ...b0}, list{ai,... bi}]

This would make a working version first, we could improve the performance later

bobzhang avatar Oct 13 '22 11:10 bobzhang

As far as I understand, after this feature is added, there will be no syntax errors related to wrong spread location, since spread expression at anywhere could be compiled to list concatenation. Do I understand correctly?

butterunderflow avatar Oct 17 '22 13:10 butterunderflow

yes

bobzhang avatar Oct 18 '22 09:10 bobzhang