-
Notifications
You must be signed in to change notification settings - Fork 38
Spread anywhere feature #692
Changes from 13 commits
0447df1
3f77897
769d719
b34ec30
3459747
cefe805
4733641
72409f1
362fca5
f14b41e
9a81c95
00226bf
ecc4d40
001501f
0e005c0
61ac172
e71a61a
839d3e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -618,6 +618,26 @@ let isTemplateLiteral expr = | |
| | Pexp_constant _ when hasTemplateLiteralAttr expr.pexp_attributes -> true | ||
| | _ -> false | ||
|
|
||
| let hasSpreadAttr attrs = | ||
| List.exists | ||
| (fun attr -> | ||
| match attr with | ||
| | {Location.txt = "res.spread"}, _ -> true | ||
| | _ -> false) | ||
| attrs | ||
|
|
||
| let isSpreadBeltListConcat expr = | ||
| match expr.pexp_desc with | ||
| | Pexp_ident | ||
| { | ||
| txt = | ||
| Longident.Ldot | ||
| (Longident.Ldot (Longident.Lident "Belt", "List"), "concatMany"); | ||
| } | ||
| when hasSpreadAttr expr.pexp_attributes -> | ||
| true | ||
|
||
| | _ -> false | ||
|
|
||
| (* Blue | Red | Green -> [Blue; Red; Green] *) | ||
| let collectOrPatternChain pat = | ||
| let rec loop pattern chain = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,79 +42,64 @@ Explanation: since records have a known, fixed shape, a spread like `{a, ...b}` | |
| 4 │ let record = {...x, ...y} | ||
| 5 │ let {...x, ...y} = myRecord | ||
| 6 │ | ||
| 7 │ let myList = list{...x, ...y} | ||
| 7 │ let list{...x, ...y} = myList | ||
|
|
||
| Record's `...` spread is not supported in pattern matches. | ||
| Explanation: you can't collect a subset of a record's field into its own record, since a record needs an explicit declaration and that subset wouldn't have one. | ||
| Solution: you need to pull out each field you want explicitly. | ||
|
|
||
|
|
||
| Syntax error! | ||
| tests/parsing/errors/other/spread.res:8:1-3 | ||
| tests/parsing/errors/other/spread.res:7:13-22 | ||
|
|
||
| 6 │ | ||
| 7 │ let myList = list{...x, ...y} | ||
| 8 │ let list{...x, ...y} = myList | ||
| 9 │ | ||
| 10 │ type t = {...a} | ||
|
|
||
| Lists can only have one `...` spread, and at the end. | ||
| Explanation: lists are singly-linked list, where a node contains a value and points to the next node. `list{a, ...bc}` efficiently creates a new item and links `bc` as its next nodes. `list{...bc, a}` would be expensive, as it'd need to traverse `bc` and prepend each item to `a` one by one. We therefore disallow such syntax sugar. | ||
| Solution: directly use `concat`. | ||
|
|
||
|
|
||
| Syntax error! | ||
| tests/parsing/errors/other/spread.res:8:13-22 | ||
|
|
||
| 6 │ | ||
| 7 │ let myList = list{...x, ...y} | ||
| 8 │ let list{...x, ...y} = myList | ||
| 9 │ | ||
| 10 │ type t = {...a} | ||
| 5 │ let {...x, ...y} = myRecord | ||
| 6 │ | ||
| 7 │ let list{...x, ...y} = myList | ||
| 8 │ | ||
| 9 │ type t = {...a} | ||
|
|
||
| List pattern matches only supports one `...` spread, at the end. | ||
| Explanation: a list spread at the tail is efficient, but a spread in the middle would create new lists; out of performance concern, our pattern matching currently guarantees to never create new intermediate data. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we may need update the docs about pattern in later commits, it is not supported since it is not deterministic |
||
|
|
||
|
|
||
| Syntax error! | ||
| tests/parsing/errors/other/spread.res:10:11-13 | ||
| tests/parsing/errors/other/spread.res:9:11-13 | ||
|
|
||
| 8 │ let list{...x, ...y} = myList | ||
| 9 │ | ||
| 10 │ type t = {...a} | ||
| 11 │ type t = Foo({...a}) | ||
| 12 │ type t = option<foo, {...x}> | ||
| 7 │ let list{...x, ...y} = myList | ||
| 8 │ | ||
| 9 │ type t = {...a} | ||
| 10 │ type t = Foo({...a}) | ||
| 11 │ type t = option<foo, {...x}> | ||
|
|
||
| You're using a ... spread without extra fields. This is the same type. | ||
|
|
||
|
|
||
| Syntax error! | ||
| tests/parsing/errors/other/spread.res:11:15-17 | ||
| tests/parsing/errors/other/spread.res:10:15-17 | ||
|
|
||
| 9 │ | ||
| 10 │ type t = {...a} | ||
| 11 │ type t = Foo({...a}) | ||
| 12 │ type t = option<foo, {...x}> | ||
| 13 │ | ||
| 8 │ | ||
| 9 │ type t = {...a} | ||
| 10 │ type t = Foo({...a}) | ||
| 11 │ type t = option<foo, {...x}> | ||
| 12 │ | ||
|
|
||
| You're using a ... spread without extra fields. This is the same type. | ||
|
|
||
|
|
||
| Syntax error! | ||
| tests/parsing/errors/other/spread.res:12:23-26 | ||
| tests/parsing/errors/other/spread.res:11:23-26 | ||
|
|
||
| 10 │ type t = {...a} | ||
| 11 │ type t = Foo({...a}) | ||
| 12 │ type t = option<foo, {...x}> | ||
| 13 │ | ||
| 9 │ type t = {...a} | ||
| 10 │ type t = Foo({...a}) | ||
| 11 │ type t = option<foo, {...x}> | ||
| 12 │ | ||
|
|
||
| You're using a ... spread without extra fields. This is the same type. | ||
|
|
||
| let arr = [|x;y|] | ||
| let [|arr;_|] = [|1;2;3|] | ||
| let record = { x with y } | ||
| let { x; y } = myRecord | ||
| let myList = x :: y | ||
| let x::y = myList | ||
| type nonrec t = < a > | ||
| type nonrec t = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ let x = list{} | |
| let x = list{1} | ||
| let x = list{1, 2} | ||
| let x = list{1, 2, 3} | ||
| let x = list{1, 2, ...x, 3, ...x} | ||
| let x = Belt.List.concatMany([list{1, 2, ...x}, [list{3, ...x}]]) | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could this be improved? print it as
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be, but just by changing the printer can cause some problems. For example, list{. . x, . .y} and Belt.List.concatMany([x, y]) will be printed as the same thing. I am still trying to solve this problem.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you add an attribute to Belt.List.concatMany to avoid accidental simplifications?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what is done for string templates, I believe, to differentiate from normal string concatenation.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check what
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for all your suggestions, I'll learn more about that. |
||
| let x = list{ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you add some more test cases, some edge cases like: let x = list{ ...xs}
let x = list{1, ...xs}
let x = list{xs, ... ys}
let x = list{ ...xs, ... ys} |
||
| superLoooooooooooooooooooooooooooooongIiiiiiiiiideeeentifieeeeeeeeeeeeeeeeer, | ||
|
|
@@ -17,3 +19,23 @@ let x = list{ | |
| superLoooooooooooooooooooooooooooooongIiiiiiiiiideeeentifieeeeeeeeeeeeeeeeer, | ||
| ...superLoooooooooooooooooooooooooooooongListHere, | ||
| } | ||
|
|
||
| let x = Belt.List.concatMany([ | ||
| list{ | ||
| superLoooooooooooooooooooooooooooooongIiiiiiiiiideeeentifieeeeeeeeeeeeeeeeer, | ||
| superLoooooooooooooooooooooooooooooongIiiiiiiiiideeeentifieeeeeeeeeeeeeeeeer, | ||
| ...superLoooooooooooooooooooooooooooooongListHere, | ||
| }, | ||
| list{ | ||
| superLoooooooooooooooooooooooooooooongIiiiiiiiiideeeentifieeeeeeeeeeeeeeeeer, | ||
| ...superLoooooooooooooooooooooooooooooongListHere, | ||
| }, | ||
| ]) | ||
|
|
||
| let x = list{ | ||
| superLoooooooooooooooooooooooooooooongIiiiiiiiiideeeentifieeeeeeeeeeeeeeeeer, | ||
| superLoooooooooooooooooooooooooooooongIiiiiiiiiideeeentifieeeeeeeeeeeeeeeeer, | ||
| ...superLoooooooooooooooooooooooooooooongListHere, | ||
| superLoooooooooooooooooooooooooooooongIiiiiiiiiideeeentifieeeeeeeeeeeeeeeeer, | ||
| ...superLoooooooooooooooooooooooooooooongListHere, | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of comment unused code, can you just remove it