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

Do collection elements null short? #394

Closed
munificent opened this issue Jun 6, 2019 · 6 comments
Closed

Do collection elements null short? #394

munificent opened this issue Jun 6, 2019 · 6 comments
Labels
nnbd NNBD related issues

Comments

@munificent
Copy link
Member

One of the explanations I've considered for which expressions null short and which don't is "We null short any expression that has a null-aware form". So we null short ., [], and ..., because we offer ?., ?.[], and ...?, respectively.

That raises a nasty question if we were to add null-aware elements (#323), which I do think we should. Does that mean single collection elements should null short?

List? list = null;
print([list?.first]);

Should this print [], and not [null]? In other words, do we treat an expression in a collection literal as an "element" that can be implicitly null-shorted if the expression is a null-aware one?

It feels pretty dubious to me since there's no syntax in there where the user has indicated they don't want the expression to simply behave like an expression. But it does mean the story that "we short anything that has a null-aware form" is no longer true if we add ? elements.

@munificent munificent added the nnbd NNBD related issues label Jun 6, 2019
@lrhn
Copy link
Member

lrhn commented Jun 7, 2019

No, single element expressions should not short-circuit to nothing if they are null, whether or not they contain a ?..

I disagree with the reasoning about where we null-short. It's not because we have operators you can use, but because it would be wrong to force you to use those operators. They do not have the same meaning as null-short circuiting.

The places where we "null short" is where we know that a null would very likely cause an error anyway, and where it's easy to reason about what will be omitted.

We are restricted by grammar because we don't have an explicit null-guard delimiter, and we don't want entire expressions to go away because of a ?. deep in the expression. That's hard to read.
For chains of member access, it's easy because you read them in the order they evaluate, left-to-right, and everything after the ?. will be dropped. That's easy to reason about, and it would likely be an error if we didn't.

For ?., if we do foo?.bar.baz without continuing the null short-circuiting, then .baz would be an error if foo is null (unless baz is really toString() or similar, but we don't want to make exceptions for those). So we skip everything down-chain from the ?..

For [], it's the same: foo?.bar[baz] is going to be an error if foo is null.

For ..., I'm still partly against extending null shortening to that because I find it too unintuitive to read. That would mean that ... foo?.bar will be an error if foo is null, and since you could write ...? foo?.bar, we could make that automatic. On the other hand, writing ...? means something else; we would implicitly allow foo!.bar to be nullable, because bar returns null, and that might not be what we intended. So, it's reasonable (many things are that we don't do), but hard to read.

In any case, for [list?.first], that is not an error, you can have null as element in a list. That expression is already valid, and we shouldn't change its behavior.
I also see no reason that expression should become an empty list and [null] should not.

Would it work for map literals too:

var map = { x?.name : y?.value };

Would this entry be elided if either x or y is null? Would it be elided if x!.name or y!.value is null?

We do not have a model where null means "nothing". There has been discussions about that, but the conclusion was that it's something different from null, and we can't treat null like that and like its current role at the same time.

What you can do is [... ifNotNull(e)] with

List<E> ifNotNull<E extends Object>(E? value) => value == null ? const <Null>[] : <E>[value];

@eernstg
Copy link
Member

eernstg commented Jun 7, 2019

I agree that the initial motivation for null-shorting was that not null-shorting produces expressions whose evaluation is guaranteed to fail (so we'd have a noSuchMethod on null event now, and a compile-time error with NNBD). So it's needed in order to avoid forcing developers to write more noisy code.

However, I don't believe that this consideration should be part of the conceptual model for null-shorting: That's about canceling an expression evaluation at a specific point, because a subexpression yielded null ("there was nothing to work on"). This makes the mechanism similar to exception handling, and I think it calls for a very simply (syntactic) criterion for delimiting the parts that we're skipping. So it should be syntactic, we have an explicit marker like ?. indicating that this kind of skipping is present at all, and the relevant syntax is always to the right (so we only skip things that are "ahead of us" when we see the ?).

If we consider making print([list?.first]); print [] then we'd have a violation of these rules: We'd presumably use the ?. to determine that null-shorting takes place at all, but then we'd extend the effect of that null-shorting to the enclosing list literal (without special shorting semantics we'd get [null]). But then the whole list literal could evaluate to null as well, because all of its elements were skipped. ;-) It's not easy to determine where to stop if we don't have an unambiguous marker to request null-shorting.

We could of course make the list literal null-shorting itself, say ?[<elements>], and this could mean that we skip every element which evaluates to null (and accept/infer a non-null element type).

This would decouple the null-shorting of the list from any null-shorting of each element (so [null, list?.first] would evaluate to [] both when list is null and when list.first is null), but it would provide a simple conceptual model for the developer: "a ?[....] filters out nulls".

Sets would work just like lists in this respect.

Maps could be null-shorting as well. @lrhn already mentioned that it's more messy, but here's a conceptual argument for omitting all bindings that contain just one null: If the value of a key/value pair is null then that key "doesn't map to anything"; if a key of a key/value pair is null then "nothing maps to that value". So we skip a binding as soon as there's at least one null. So ?{null: 1, true: null} is the empty Map<bool, int>. Again, null-shorting could of course occur in each key and value as well, but that's just gives us more ways to obtain the value null, it doesn't change the treatment of nulls in the enclosing map.

It would still make sense to have null-shorting for each element of a set/list as well, cf. #323, especially considering the amount of machinery that @lrhn demonstrated we'd need with ifNotNull, and null-shorting sets and lists could then easily be desugared by putting a ? in front of each element whose type is potentially nullable.

@munificent
Copy link
Member Author

I don't disagree overall, but I don't think this argument is entirely consistent either:

I think it calls for a very simply (syntactic) criterion for delimiting the parts that we're skipping. So it should be syntactic, we have an explicit marker like ?. indicating that this kind of skipping is present at all, and the relevant syntax is always to the right (so we only skip things that are "ahead of us" when we see the ?).

By that token, we should also null-short both:

a?.b + c
a?.b ? c : d

Both of those:

  1. Will almost definitely produce an error if we don't null short them.
  2. Have a subexpression that yields null ("there was nothing to work on").
  3. Have an explicit marker.
  4. The part skipped is to the right.

In the case of +, it literally is just a method call so it has the exact same semantics as a?.b.plus(c), which is shorted.

I'm not saying we should short these (I think we probably shouldn't), just that we still don't have a simple explanation for why some things are shorted and these are not.

We could of course make the list literal null-shorting itself, say ?[<elements>],

This would collide with an explicit null-aware element (#323):

var list = [
  ?[null].length
];

Does that produce [0], or [1]?

@eernstg
Copy link
Member

eernstg commented Jun 10, 2019

@munificent wrote:

we should also null-short ..

Using a syntactic rule would just imply that the decision is made without reference to context dependent information (like the static type of an expression). So that doesn't tell us anything about whether we should null-short across a + or a ?:. The point is that it should be an easy and safe operation for the reader of a piece of code to determine where null-shorting stops, and I think that's going to be a lot more difficult if you can't answer that question without knowing the types of each subexpression of a complex expression.

Concretely, I suspect that null-shorting across + or ?: is a bad idea, simply because it's more difficult to see where null-shorting ends when it's supported for larger expressions, so I prefer the approach in PR 293 which is basically focused on chains of selectors and cascades. We could of course go as far as we want, but I think that stopping there is a reasonable way to keep it simple.

For the ambiguity with ?[....], we'd need to disambiguate, but I think we can do that, having practiced that art recently. ;-) For instance, [ ?[null].length] could evaluate to [1] (with a hint that the ? is superfluous because [null].length will have a non-nullable type), and the other interpretation could be forced by using [ (?[null].length)].

@leafpetersen
Copy link
Member

Reading the comments on this, I don't see a consensus for doing this. I will close this out unless anyone has more they want to say here.

@munificent
Copy link
Member Author

I think there is consensus that we should not do this, if I'm reading the comments right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nnbd NNBD related issues
Projects
None yet
Development

No branches or pull requests

4 participants