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

Spread Collections #47

Closed
munificent opened this issue Oct 15, 2018 · 55 comments
Closed

Spread Collections #47

munificent opened this issue Oct 15, 2018 · 55 comments
Assignees
Labels
feature Proposed language feature that solves one or more problems
Milestone

Comments

@munificent
Copy link
Member

munificent commented Oct 15, 2018

Solution for #46.

Feature specification.

Most other languages have a "spread" or "splat" syntax that interpolates the elements of an existing collection into a new collection. In JS, it's a prefix .... In Ruby, Python, and a couple of others, it's a prefix *.

I propose we follow JS and use ....

This proposal is now accepted. Implementation work is being tracked in #164.

@munificent munificent added the feature Proposed language feature that solves one or more problems label Oct 15, 2018
@munificent munificent changed the title Spread operator in collection literals Spread Collections Oct 15, 2018
@zoechi
Copy link

zoechi commented Oct 16, 2018

Do you think there is place in this proposal to also consider
flutter/flutter#17862
where null values are dropped when spread?

@munificent
Copy link
Member Author

I don't think it's a good idea to silently discard null values. That might be the right thing for some APIs, but others may consider null values to be useful and meaningful. If the spread syntax always discarded them, it means you could never use a spread with those APIs.

@zoechi
Copy link

zoechi commented Oct 17, 2018

I meant perhaps something like null-aware operators combined with spread to ignore null values, not ignoring them in general.
Just thought it might make sense to consider this use case when designing the spread feature.

@zoechi
Copy link

zoechi commented Oct 17, 2018

I guess an extension method/getter like someList.nonNullValues would do anyway

@cbazza
Copy link

cbazza commented Oct 19, 2018

Just keep in mind that when designing a spread operator it goes hand in hand with the syntactically identical rest operator which goes hand in hand with destructuring syntax ;)

@munificent
Copy link
Member Author

Yes, the proposal doesn't mention that, but it was designed with destructuring and rest parameters in mind.

@munificent
Copy link
Member Author

munificent commented Oct 25, 2018

@alorenzen
Copy link

AngularDart makes heavy use of const lists as part of our configuration for the template compiler. In order to support items stored in another const list, we allow a "recursive" union type:
List<T | List>

According to your proposal, Spread elements are not allowed in const lists or maps.

Could this be adjusted to allow const elements to be spread in const lists?

For example,

const foo = [Foo];

static bar = [Bar];

const containingFoo = [
  ...foo, // Allowed
  ...bar, // Not allowed
];

@xster
Copy link

xster commented Oct 25, 2018

I don't know if it's the right protocol but what @zoechi pointed out is a common Flutter issue (though it may need an orthogonal category of solution). Branching that thread to #62

@nshahan
Copy link

nshahan commented Oct 25, 2018

It seems to me that there is a missing option in the discussion about the null aware spread operator.

In the the postfix version could the ? appear before the ...?

[foo?.bar?...]
// ^ |  ^   |
// '-'  '---'

In my eyes this seems more consistent with the existing null aware operator since the question mark appears directly after the value being checked for null.

@dnfield
Copy link

dnfield commented Oct 25, 2018

  1. One potential source of confusion for JavaScript developers is that this syntax will apply to lists and maps, but not objects.
  2. Can it apply to Iterables? If so, reading more closely, it looks like any iterable should work, so the points raised by @zoechi @xster become:

...listWithNullValues.where((entry) => entry != null),

I don't see this as being so burdensome that we should be trying to get a operator that does it automatically - and something doing it automatically could be very confusing to people who do want to preserve nulls in there.

@munificent
Copy link
Member Author

Could this be adjusted to allow const elements to be spread in const lists?

Not easily, unfortunately. Consider:

class InfiniteSequence extends ListBase<int> {
  const InfiniteSequence();

  Iterator<int> get iterator {
    return () sync* {
      var i = 0;
      while (true) yield i ++;
    }();
  }
}

const List<int> things = InfiniteSequence();
const forever = [...things];

The static type system has no way of knowing things isn't a "real" list that could be safely spread. Any subtype of a List is a List as far as it knows. So in order to prohibit code like this, we'd have to introduce some extra notion of constness to the type system, which would be a huge change.

Const is just really annoying and limited in Dart, unfortunately.

In the the postfix version could the ? appear before the ...?

Yes, that's an option too. But, in general, I and the language leads prefer the prefix syntax.

One potential source of confusion for JavaScript developers is that this syntax will apply to lists and maps, but not objects.

Dart doesn't have objects in the JS sense, just maps, so there shouldn't be too much room for confusion.

Can it apply to Iterables?

Yup, you can spread any object that implements Iterable. The .where() example you show would work fine. :)

@leafpetersen
Copy link
Member

Could this be adjusted to allow const elements to be spread in const lists?

Not easily, unfortunately. Consider:

We actually could do this, I think, but it adds another annoying special case to consts. Basically you just say that spreads can appear in const lists if the target of the spread evaluates to a const element of the builtin list type.

@munificent
Copy link
Member Author

munificent commented Oct 25, 2018

We actually could do this, I think, but it adds another annoying special case to consts.

Const is nothing but a collection of annoying special cases. :)

Basically you just say that spreads can appear in const lists if the target of the spread evaluates to a const element of the builtin list type.

Oh, right, because we do have access to the actual const value at compile time. That's a good point. Do you think it's worth adding to the proposal? I'm agnostic since I basically never use const anyway.

@leafpetersen
Copy link
Member

Do you think it's worth adding to the proposal? I'm agnostic since I basically never use const anyway.

How about opening an issue for discussion and feedback of this (link it here)?

@munificent
Copy link
Member Author

Done! If you'd like to talk about const spreads, go here: #63.

@lrhn
Copy link
Member

lrhn commented Oct 26, 2018

The list-spread feels similar to string-interpolation: I'm writing a literal X, and I want to embed another X in it. I think it makes perfect sense from that perspective.
It may look like rest parameters/spread arguments, but it is actually a separate thing if you see it as being about literals rather than mere repetitions.

You can introduce a computed sequence by using IAFLs (Immediately Applied Function Literals):

var list = [
  something, 
  other,
  ... () sync* { 
    if (someTest) yield someValue;
    if (otherTest) yield* otherValues;
    List complexList;
    complexComputationPopulatingList; 
    yield* complexList;
  } (),
  whatnot
];

If that use-case is common, doing complex computation in the middle of building a literal, we could perhaps introduce a shorthand for it, to avoid the () sync* ... () overhead.

@cbazza
Copy link

cbazza commented Oct 26, 2018

How about spreading a map into a constructor? to help cleanup long Flutter widget build methods.
https://spark-heroku-dsx.herokuapp.com/index.html

@dnfield
Copy link

dnfield commented Oct 26, 2018

What would the result be of ...[null, null].where((entry) => entry != null)? Just a null?

@dnfield
Copy link

dnfield commented Oct 26, 2018

Or ...null for that matter

@munificent
Copy link
Member Author

How about spreading a map into a constructor?

That is a much harder problem because it interacts with the static type system. A map is homogeneously-typed in Dart. A set of named arguments are not. For named arguments, what you really need is something more like a [record type](https://en.wikipedia.org/wiki/Record_(computer_science\)).

If we had rest parameters, we could support spreading to those in a fairly straightforward manner. This proposal includes that. However, rest parameters are a big complex feature because they affect the function calling convention. This proposal is much simpler since the compiler can desugar the spread to a set of operations on a list or map.

What would the result be of ...[null, null].where((entry) => entry != null)? Just a null?

The ... isn't an operator that you can use as an expression. It's part of the collection literal syntax, so it can only be used before an element in a list or map. In those cases, that code would insert no elements in the collection.

@rrousselGit
Copy link

That is a much harder problem because it interacts with the static type system.

This should however be possible with classes instead of maps right?

class Foo {
  String foo;
}

function({ String foo }) {}

/* TEST */

final Foo foo;
function(...foo);

@munificent
Copy link
Member Author

Ah, interesting. Yes, in theory we could treat the getters on a class as defining an ad-hoc record type and use that to destructure to the named parameters. That feels pretty dubious to me. If we're going to start supporting destructuring for classes, I think class authors should have more control over how that destructuring works.

@rrousselGit
Copy link

rrousselGit commented Oct 26, 2018

I think this is a pretty important aspect of the destructuring as Flutter uses mostly named parameters and classes properties.

SliverPersistentHeader or ThemeData would benefits directly from this.

Using JS as inspiration we could have the following:

class Foo extends StatelessWidget {
  final int omitted;
  final int foo;
  final int bar;

  build() {
    final { omitted, ...other} = this;
    return Widget(
     ...other,
    );
  }
} 

@munificent
Copy link
Member Author

But this is an operator, comparing it to methods isn't really fair.

The other operators also do not ignore null:

main() {
  null && false; // Runtime exception.
  true && null;  // Runtime exception.
  null || false; // Runtime exception.
  false || null; // Runtime exception.
}

Since non-nullable types have been brought up, it's worth discussing how they would interact with this. The intent of non-nullable types is to let you catch incorrect null reference errors at compile time. If we make ... always ignore null, then it should logically allow a nullable expression type, since its behavior is well-defined on null.

But if we do that, then there's no way to statically catch cases where you are using ... on something that you don't intend to be null. Since it silently accepts nullable types, there's no type you can use to get it to yell at you if you try to spread something that should not be null.

@Hixie
Copy link

Hixie commented Nov 8, 2018

This isn't really an operator. It's more akin to "async" or "yield" (both of which don't ignore null, they just pass it through).

I don't really object to ... throwing when given null as the general case of it throwing when given a non-Iterable. What I object to is ...? as a prefix syntax. I merely offer special-casing null for ... as a way to simultaneously address the concerns that have been raised about ...?foo?.bar and the requirement that we use a prefix syntax. If the requirement that ... not special-case null is stronger than the requirement that we use a prefix syntax, then we could just use suffix syntax, which solves the problem of ...?foo?.bar by sacrificing the prefix requirement.

@cbazza
Copy link

cbazza commented Nov 8, 2018

I highly prefer prefix syntax;
for the ...?foo?.bar case, use parenthesis ...?(foo?.bar) to make things clear.
You can always write confusing code with any syntax.

@munificent
Copy link
Member Author

There is another potential solution. One thing we've discussed is making the null-aware operators short circuit. It's stupid and pointless that once you use one null-aware operator, you have to use them for the rest of the method chain. Code like this is always wrong:

foo?.bar.baz().bang();

You have to write:

foo?.bar?.baz()?.bang();

The obvious fix (which C# has always done) is to short-circuit the rest of the method chain if the LHS of a null-aware operator returns null. That would make the first example behave like the latter.

If we do that, we could also make ...? short-circuit if the spread expression is a method invocation (or chain of invocations). So you'd never have to write:

[...?foo?.bar()]

Instead, it would be:

[...?foo.bar()]

And the reader knows the first ? means "if foo returns null, skip the rest of the expression and don't insert anything".

@rrousselGit
Copy link

rrousselGit commented Nov 8, 2018

Code like this is always wrong:

foo?.bar.baz().bang();

While I like the idea, it faces the same issue then with allowing ...null.
With non-nullable types, the code from above can become perfectly valid and we end up implicitly testing nulls.

It also makes no sense to allow foo?.bar?.baz if we can write foo?.bar.baz

Alternatively, it is a breaking change.

@lrhn
Copy link
Member

lrhn commented Nov 12, 2018

I think making ...? short-circuit on the first ... something ... is unworkable, mainly because it's not clear what the "first" thing of a sequence is.

[...? foo.bar.baz()]

Should this check whether foo is null, whether foo.bar is null, or whether foo.bar.baz() is null.
If we need anything, it's the last one. There is no workaround for that, the other two can be handled by inserting ?..
If we look only at the "first" thing, if foo is a prefix ... how does it read?

(I'd also use a space after the ... and ...? which helps readability in all cases).

@rrousselGit
Copy link

I probably speaks too much, but:

Would cascade make sense with spread too?
We can think of this:

List<String> foo = [
  "before",
  Foo()
    ...bar
    ...baz,
  "after",
];

which would be equal to

Foo tmp = Foo();
List<String> foo = [ "before"]
  ..addAll(tmp.bar)
  ..addAll(tmp.baz)
  ..add("after");

This means that instead of

...?foo?.bar

we could do

foo?...bar

@munificent
Copy link
Member Author

I think making ...? short-circuit on the first ... something ... is unworkable, mainly because it's not clear what the "first" thing of a sequence is.

On further thought, I agree. Perhaps the right way to handle this is that if we add general-purpose support for short-circuiting to ?. then to include a spread in the scope of what gets short-circuited. That would let you do:

[...foo?.bar()]

So you'd never need ...? if the expression being spread was itself a null-aware method chain. You might still want it for other non-null-aware expressions that could be null. But that avoids the gnarly mixture of both ...? and ?..

@lrhn
Copy link
Member

lrhn commented Nov 14, 2018

Generalizing wildly, we can allow ? between any operator and its operand(s?), both prefix, postfix and infix. Currently we treat .selector as a suffix operator.

Then you can write:

var x = null;
!?x;  // null or boolean negate if not null
-?x; // null or negate
x ?+? x;  // check both for null, otherwise add?
x?++;  // null or increment.
x?[42];  // null or index

Then ...? would be completely consistent use of ? on a prefix operator.

It's ... unlikely to be readable in general, though, and will likely clash with the conditional expression in some way.

(It's also unclear whether e1?+?e2 is short-circuiting the evaluation of e2 if e1 is null).

@cbazza
Copy link

cbazza commented Nov 14, 2018

@lrhn

Generalizing wildly, we can allow ? between any operator and its operand(s?), both prefix, postfix and infix. Currently we treat .selector as a suffix operator.

The problem is that ? is used to mean something else and it creates syntactic ambiguity, like for example:

x?[42]; // null or index

Conditional expression
x ? [42] : null

@munificent
Copy link
Member Author

var wat = { x ? [42] : y };

Does this create a map or a set? :)

@lrhn
Copy link
Member

lrhn commented Nov 23, 2018

As I said: "wildly" :)

The [...] index operator is usually the problem, and the reason I normally suggest foo?.[bar] as the null-aware index operator instead of foo?[bar], similarly to how it's used with cascade: foo..[bar]. (I usually also say that other operators must be treated like methods when used with null-aware access, like foo?.+(bar), and that can genralize to tear-offs like var fooPlus = foo.+;).

So, still speculating wildly, I think I have a solution for extending null-awareness of the receiver to operators, what we lack is null-awareness of the other operands. One (again quite overloaded) option is to allow ? as a prefix on arguments so that foo(?bar) will evaluate foo and bar, but then if bar is null, it evaluates to null without calling foo.

It's not symmetric. It doesn't generalize to operator operands (unless we go foo.+(?bar) for that, which is straining readability even further). It just might work :)

@cbazza
Copy link

cbazza commented Nov 23, 2018

// Just for reference, check out Swift's operator creation functionality... very powerful.
https://medium.com/@vialyx/swift-course-advanced-operators-1c86f4ae7d67
https://docs.swift.org/swift-book/LanguageGuide/AdvancedOperators.html

@mit-mit mit-mit added this to Being spec'ed in Language funnel Nov 28, 2018
@mit-mit mit-mit moved this from Being spec'ed to Ready for implementation in Language funnel Dec 1, 2018
@mit-mit mit-mit moved this from Ready for implementation to Being implemented in Language funnel Jan 16, 2019
@Solido
Copy link

Solido commented Feb 10, 2019

Very secondary question but why ... instead of *.
Quoting the documentation :

The expression following yield* must denote another (sub)sequence ...

In a sync* function, the subsequence must be an iterable ...

So in my mind * is binded to sequence and then when spreading it goes to ... that is the JS expression.

Is there any reason for having two symbols for the same concept of sequence ?

@lrhn
Copy link
Member

lrhn commented Feb 11, 2019

The * is less conspicuous and easier to confuse with multiplication.
It's not impossible to see things like:

var list = [
  something,
  someLongValue
  * example;
];

Here I forgot the , after someLongValue, and that turned the expression into a multiplication.
The ... is not used anywhere else, and it stands out when looking at the code.

I can see that yield* uses * as sequencing, and we were actually toying with using yield and yield* in the syntax. It just wasn't worth it.
Also, I might also be damaged by a history of C programming, where *list means something quite different.

So, in short, the ... for spread is used in multiple languages, and it isn't generally used for something not related to sequences. The * is used for spreading in multiple languages, but also with different meanings in both Dart and other languages, and it's easier to make mistakes with it.
The ... syntax had about the same number of pros, and fewer cons, than *.

@aartbik
Copy link

aartbik commented Feb 21, 2019

@aartbik

dart-bot pushed a commit to dart-lang/sdk that referenced this issue Mar 8, 2019
Rationale:
Implementation of control-flow collections is
done through the notion of block expressions.
This is a first implementation to get this
working. It takes a few shortcuts (like disabling
OSR inside block expressions for now) that may be
refined later.

dart-lang/language#78
dart-lang/language#47

Change-Id: I966bf10942075052fcfd9bac00298a179efc551b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/94441
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
@mit-mit mit-mit moved this from Being implemented to Done in Language funnel May 1, 2019
@mit-mit
Copy link
Member

mit-mit commented May 1, 2019

Closing; this is launching in Dart 2.3

@mit-mit mit-mit closed this as completed May 1, 2019
@mit-mit mit-mit added this to the Dart 2.3 milestone May 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
Development

No branches or pull requests