Reason: List merging

Created on 3 Dec 2016  路  15Comments  路  Source: reasonml/reason

Right now there is syntax for appending a list to a new list, but it would be nice to have a way to easily merge lists in different places.

let one_two = [1, 2];
let four_five = [4, 5];
let merged = [0, ...one_two, 3, ...four_five];
FEATURE REQUEST Parser

Most helpful comment

It just seems inconsistent that I can do:

{ ...previous, prop: val }

But not

[ ...previous, val]

Even noting that those are very different operations

All 15 comments

cc @jordwalke. We could but decided against it because it's a bit too easy to use it and not realize the performance penalty. OCaml lists are old school singly linked list, so spreading at head is O(n).

My argument for supporting this is because usually, if I wanted to join two lists, I've have done it anyway through foo @ bar (which this spread actually desugars too btw). But I'm ambivalent now.

Edit: how ironic, I screwed up. [1, ...foo] desugars to 1::foo which is O(1) append at head. Guess that makes jordan's argument slightly more legit lol.

Yeah, we're open to the idea (we want something like it for JSX as well). But it would be great if people would understand the performance costs of various operations. Appending an item onto the head of a list is fundamentally different than concatenating/spreading. In JavaScript, everything is O(n) (to concat two lists), so the distinction doesn't matter much - all joining operations are expensive (and access is O(1) fast).

It just seems inconsistent that I can do:

{ ...previous, prop: val }

But not

[ ...previous, val]

Even noting that those are very different operations

I think we should support this feature -- It should be very trivial to understand the performance implication.

Some care should be taken to ensure that the desugared version is tail recursive (List.concat and @ are not) or that it's very clear to the user that this syntax will cause a stack overflow for larger lists.

It does seem odd and a bit of a waste to offer special syntax then immediately turn around and say "this is a bad idea because of performance." Would it be possible to support another data structure for this syntax? Something rope-like wouldn't have quite the same performance penalty as list concatenation. Or perhaps arbitrary data structures support. There is upstream OCaml work which may or may not land to support custom indexing operators. That could play nicely with this syntax proposal.

If this sugar is added, please make it clear what function is actually called, so that I can override it with one that immediately raises to help ensure that no-one mistakenly uses it. Appending long lists is always wrong, and appending short lists silently turns into appending long lists over time.

@jberdine I also appreciate the performance-by-default design of OCaml and it shows here. I think if we create a sugary list concat, it should be visually distinguished from constant time operations somehow. So we would want something different than [hd, ...concated, ...tl].

Maybe something like [...@previous, val]

Just suggest that syntax when the compiler errors out for discoverability and it's perfect 馃槈

@thejameskyle That's actually a great idea! When someone tries to try the "slow thing", we can parse it, and even generate a compiler error (by simply inserting an "extension point" node with the error text), but a very helpful and specific one that explains what they should write instead. I'll mark this as a #GoodFirstTask

To whoever takes this on: See how the special --recoverable mode of refmt works. It replaces failed parse regions with something called "extension point nodes" (which you could write manually as well via [%hey I am an extension node]) but we insert them automatically so that you get nice errors in the editor with Merlin. Well for this task, you would even insert those when not specifying --recoverable to refmt, whenever you detect [x, ...foo, y] where ...foo is no the last item in the comma separated list.

FWIW, I'll echo the sentiment that we should not provide syntax for operations which are not tail recursive. Regarding the spread operator, it's reasonable for a consumer of this API to assume it will be O(N), but providing operators which result in stack overflows when N is > 1000 seems irresponsible. Frankly, I think these functions should not even be in the standard library.

That said in Issue 971 I propose adding the spread operator for Arrays as well, which is generally O(N). This would be particularly valuable given what a pain writing such code with blits by hand can be.

Merging #1037 into this

E.g. [...xs, x] causes SYNTAX ERROR on the comma, and if you're a bit dense more familiar with javascript it can take a while to see problems like this.

The only supported list spread syntax is [x, ...xs] because it will be desugared into ocaml x :: xs before optionally being transpiled to javascript. Which isn't quite as obvious for those who use reason more as a syntax flavor of javascript, and therefore don't care as much about the middle ocaml step and the limitations that imposes.

1037 seems like a good argument for making different things look different. ... and :: are not the same.

I think this would be a nice addition... I do agree that it could kinda hide the fact that there is a performance penalty, but 99% of the time it has no relevance.
Also if im doing [...xs, x] its usually because I want x to be at the end of the list... Its true that putting x at the front would be more efficient, but it does not solve my problem.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bluddy picture bluddy  路  3Comments

gustavopinto picture gustavopinto  路  3Comments

jberdine picture jberdine  路  3Comments

ostera picture ostera  路  3Comments

chenglou picture chenglou  路  3Comments