dart-lang / language

Design of the Dart language
Other
2.66k stars 205 forks source link

Spread Collections #47

Closed munificent closed 5 years ago

munificent commented 6 years ago

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.

zoechi commented 6 years ago

Do you think there is place in this proposal to also consider https://github.com/flutter/flutter/issues/17862 where null values are dropped when spread?

munificent commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

cbazza commented 6 years ago

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 commented 5 years ago

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

munificent commented 5 years ago

The proposal for this has now landed: https://github.com/dart-lang/language/blob/master/working/spread-collections/feature-specification.md

alorenzen commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago
  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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

lrhn commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

dnfield commented 5 years ago

Or ...null for that matter

munificent commented 5 years ago

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.

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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,
    );
  }
} 
Hixie commented 5 years ago

We were talking about the ...?foo?.bar problem at lunch today. I still think that's so horrifically bad that it should make the prefix version of this syntax a non-starter. However, @dnfield had understood your response above as saying that ...null should insert nothing, meaning that ... is actually null-aware on its own, and ...? is not necessary. That seems great to me. If it is indeed the case that there is no separate ...? syntax because ... on its own is already null-aware, then I retract my objections to the prefix ... syntax.

cbazza commented 5 years ago

I was under the impression that null-awareness was related to the contents of what was going to be spread and not the collection itself. If the collection itself is null than there is nothing to spread.

If foo?.bar returns null, the collection is null so either ... or ...? will insert nothing right?

but if foo?.bar returns [2, null, 3], ...foo?.bar will return [2, null, 3] whereas ...?foo?.bar will return [2,3].

Since what comes after ... or ...? is an expression, you can always use parenthesis to clarify things: ...?(foo?.bar)

To me spread is 'much better' in prefix form.

dnfield commented 5 years ago

It would be most helpful to me to see something like this documented:

final List<int> listOneToFive = <int>[1, 2, 3, 4, 5];
final List<int> nullList = null;
final List<int> listOfNulls = <int>[null, null];

print(listOneToFive); // '[1, 2, 3, 4, 5]'
print(nullList); // null
print(listOfNulls); // '[null, null]'

print([0, ...listOneToFive, ...nullList, ...listOfNulls]); 
  // is it A) '[0, 1, 2, 3, 4, 5, null, null]'
  // B) '[0, 1, 2, 3, 4, 5, null, null, null]'
  // C) exception (NPE or something like that)?

I would expect to see the first option listed (A)).

munificent commented 5 years ago

However, @dnfield had understood your response above as saying that ...null should insert nothing, meaning that ... is actually null-aware on its own, and ...? is not necessary.

My response above was that ...[null, null].where((entry) => entry != null) inserts nothing. That's because [null, null].where((entry) => entry != null) evaluates to an empty iterable. Spreading an empty iterable inserts nothing. There is no implicit discarding of nulls. In @dnfield's example, the .where((entry) => entry != null) is explicitly filtering them out.

Documenting this explicitly is a good idea. I'll update the proposal. For your immediate edification:

print([1, ...null], 2);    // Runtime exception, calling ".iterator" on null.
print([1, ...[], 2]);      // [1, 2]
print([1, ...[null], 2]);  // [1, null, 2]

print([1, ...?null, 2]);   // [1, 2]
print([1, ...?[], 2]);     // [1, 2] (same as "...")
print([1, ...?[null], 2]); // [1, null, 2] (same as "...")

Edit: PR: https://github.com/dart-lang/language/pull/86

Hixie commented 5 years ago

Since there is no value in spreading null with the null-unaware syntax (it always crashes), is there any reason to not just always make ... null-aware? Especially assuming a future will non-nullable variables?

munificent commented 5 years ago

Since there is no value in spreading null with the null-unaware syntax (it always crashes)

Crashes (well, runtime exceptions with useful stack traces and descriptive messages) are the value. If you don't expect the expression to evaluate to null, it tells you exactly where that's happened.

is there any reason to not just always make ... null-aware?

As the proposal says, it's consistent with the rest of the language and even spread in JS where null is not silently ignored. We don't treat null as false in an if condition, while condition, conditional expression, or do while loop. We don't ignore it if you pass null to List.addAll() or StringBuffer.writeAll(). We don't discard it in an interpolated string.

In all other places in the language, null is a real and present value.

Especially assuming a future will non-nullable variables?

If we have non-nullable types, then the idea is that you'll need to use null-aware spread if the expression you're spreading is nullable. Otherwise, you'll get a static error. That helps you catch unintended unsafe operations at compile time. If a null is intended, you can make it safe using ...?.

This is also consistent with how I expect we'll handle null in other places like if conditions, List.addAll(), etc.

Hixie commented 5 years ago

I don't really see the value in ... crashing when given null, but ok.

If we are having the null-aware spread syntax then I continue to object strenuously to ...?foo.bar?, which I think is horrific. It's inconsistent with the rest of the language (which puts the ? after the thing being null-checked), and is extremely confusing (I honestly don't think anyone will ever guess what it means on a cold reading).

rrousselGit commented 5 years ago

Is the ...? actually necessary?

If we have ...? I'd find it weird to have both ?. and ...? but ..? not being a thing.

xster commented 5 years ago

Tangential but fwiw, a ?.. would be nice to have https://github.com/dart-lang/sdk/issues/30541

dnfield commented 5 years ago

@rrousselGit imagine the following:

  List<Widget> extraChildren = getExtraChildrenOrNull();
  return Column(
    children: <Widget>[
      Text('Always first'),
      ...?extraChildren,
      Text('Always last'),
    ],
  );

But I agree that the ...? feels weird and there are legitimate cases that will be difficult to parse out (...?object?.prorperty just doesn't look right). I would probably prefer using

  List<Widget> extraChildren = getExtraChildrenOrNull();
  return Column(
    children: <Widget>[
      Text('Always first'),
    ]
    ..addAll(extraChildren)
    ..addAll(Text('Always last')),
  );

which feels heavy.

rrousselGit commented 5 years ago

With only ... we can do the following:

...(foo ?? const [])

This is less optimized character wise, but it's something a Javascript developer will find more natural. As with JS we'd handle null spread as such :

...(foo || [])

rrousselGit commented 5 years ago

If the argument of being familiar to Javascript doesn't count, then one counter argument to allowing ...null disappear.

It leaves us with :

other sdk methods don't ignore null

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

As an operator I think it's fine for it to have a different behavior.

lrhn commented 5 years ago

Other operators also do not ignore null, unless they are explicit null-aware operators. Well, unless they are exactly ?., the other null aware operators, ?? and ??=, don't actually ignore null.

Comparing operators to methods is fair if you want a consistent treatment of null across the entire programming experience. The way Dart treats null is that it is never considered equivalent to an actual value. It is not false if you expect a bool, it is not the same as an empty string if you expect a string, or a list. Instead it is always treated as an error when you try to use it in a way that isn't available to all objects. The "null aware" operators are shorthands for adding null-tests, like you would otherwise do to avoid doing operations on null.

So, for ... to treat a null as equivalent to an empty iterable is at odds with how null is treated everywhere in the Dart language and platform libraries.

People ask for non-nullable types in order to avoid null-errors only being caught at run-time. Having an operator that works on a nullable type by ignoring the null will hide the kind of errors those people are explicitly trying to catch.

rrousselGit commented 5 years ago

Good point on the if (null), in that case I agree. Consistency is important

munificent commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago
var wat = { x ? [42] : y };

Does this create a map or a set? :)

lrhn commented 5 years ago

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 :)