! should partipicate in null shorting
#1163
Comments
Postfix increments and decrements are null-shorting expressions (e.g. `x?.y++.isEven` only calls `isEven` if `x` was non-null), but it is still being decided whether postfix `!` should be null shorting (i.e. whether `x?.y!` should fail if `x` is null). See dart-lang/language#1163. Previously, the analyzer was inconsistent about whether it considered `!` to participate in null shorting; as a result, when analyzing an expression like `x?.y!`, the analyzer would fail to call `FlowAnalysis.nullAwareAccess_end`, resulting in corrupted flow analysis state, which could lead to a crash. This change makes the analyzer treat `!` as *not* participating in null shorting, which is consistent with what is currently written in the spec and implemented in the CFE. Fixes #43093. Change-Id: Ie69c5c29f226fe1a0282d0e7a1e079778dc700c3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/159147 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
|
I worry that users will find the resulting type of the expression to be very unintuitive. A couple of examples: expectInt(int i) {}
test(List<int?>? list) {
expectInt(list?.first!); // Error.
list?.first! + 2; // Error.
var x = list?.first!;
x.isEven; // Error.
}So the user is explicitly writing One option would be to only null short list?.first! // Equivalent to: `(list?.first)!`
list?.first!.isEven // Equivalent to: `(list != null) ? (list.first!.isEven) : null`In other words, we short |
|
What about making it an error to apply Alternatively, say that it does participate in null shorting, but make it a warning to avoid the confusion that @munificent points out (which I agree is really bogus - it's really surprising that an expression of the form |
Personally I'm not a huge fan of this. I buy Lasse's argument (from our meeting this morning) that allowing
I like this, assuming that when you say "make it a warning", you mean "make it a warning when a null-shorted @bwilkerson @pq for visibility on the warning/quick fix idea. |
For situations like this - if we don't do null shortening then it really converts it to |
|
Ok, let me try to pull together some pieces here. Suppose we have: class A {
int get zero => 0;
int? get zeroOrNull => null;
}Suppose we have a local variable Consider
Consider
Consider
Consider
Summarizing, if we agree with my evaluations above... let's say that the user falls into the pit of success if they get the interpretation they expected, the pit of non-failure if they get an error, and the pit of failure if they sometimes get the wrong behavior, and the pit of despair if the always get the wrong behavior. If
If
Does that summary seem fair? The |
|
@munificent If a user writes @leafpetersen The summary seems fine. It's worth noticing that the two Failure entries are both where the intention is unknown. That is, we don't know whether it's a failure or success. I'll still argue that consistency and simplicity are sufficient to choose
Also, if |
|
@leafpetersen wrote:
It looks like there's a rather strong argument in favor of However, I agree with @lrhn that it is more consistent to let But even more, we can't say that we are avoiding the parentheses: Being a selector, I believe we can't change Next, I'm not convinced that a developer who writes The same consideration applies to What could the intention be for With So I'd still prefer that However, should we then emit a hint in the case where |
|
@leafpetersen, thanks for writing up all of the interesting cases. When I look at them, the question I find myself asking is, "If a user wrote that, would it be useful?" By that I mean:
The only two cases that I see being useful—where I would be happy to see a user write them—are: a?.zeroOrNull!
a?.zeroOrNull!.isEvenAnd they are only useful if If we short, the other two cases become pointless but harmless: a?.zero!
a?.zero!.isEvenFortunately, they give you a static warning letting you know that what you wrote is pointless. On the other hand, if we don't short, every one of these cases becomes what I'd consider bad code. There's always a better way to express the same thing: a?.zero! // Should instead be: a!.zero
a?.zeroOrNull! // Should instead be: a!.zeroOrNull!
a?.zero!.isEven // Should instead be: a!.zero.isEven
a?.zeroOrNull!.isEven // Should instead be: a!.zeroOrNull!.isEvenI see a compelling argument that shorting is the right behavior. I was worried that it would be hard to explain why an expression that textually ends with |
|
So, oddly, I find myself becoming a hold out here.
I actually think this is the wrong question, or at least, only one of the questions that needs to be asked. The question I find myself asking is "what did the user most likely intend?". And I cannot convince myself that a user who writes
I strongly doubt this statement. I'm fairly strongly of the opinion right now that if we do this, we should make it an error to terminate a null-shorting expression with a I think that if we allow it to be the case that an expression of the form |
|
Here's a similar story:
Writing We can still hint in the cases like |
|
Summary: I firmly believe that (Basically: What Bob said!)
The analyzer also yells at them for using a
I definitely do not want Then we can still make not make a trailing Spreading If we let a trailing If we make a trailing If we start disallowing useless things, I think we should be consistent about it, otherwise it's just a weird wart on the language, not a useful feature. If the user gets confused because adding
We expect users to recognize that because they know how I want users to think of
|
I tested this out on a couple of people last night, neither of them saw this as an option, and neither of them did this. |
No it doesn't. |
No, this is not about precedence. Precedence is entirely irrelevant to this discussion. |
Those are all places where it's very clear that there is an ambiguity going on, and there is a precedence issue here. I expect users to either know about precedence, or use parenthesis, when there is a precedence issue. We are not discussing a precedence issue. We are discussing how the continuation passing transform which gives semantics to null shorting treats |
I want many things that I can't have. I think this is an unrealistic desire, and not even a very sensible one. Even if you look at the grammar, it's clear they are difference. A cascade consists of a series of cascade sections. Each section is a collection of selectors (I'm speaking very roughly here). It's reasonably natural to see that |
|
Here's another way of talking about what's going on. We don't have a Given that we have not done so, we're in a bit of a pickle. You can simulate the Well, one thing you could do is encode the difference into the null shorting rules (do a different thing with a terminating Another thing you could do is just not allow Are there other things we can do here? |
|
For the record, I do not think a participating final I do think a non-participating final Disallowing a final I can't think of a fourth thing to do about Knowing how far a Making a trailing (I use "precedence" about any choice about which syntax binds stronger, not just binary operators, so whether |
I think this gets to where we're talking past each other. :) I'm not claiming it's useless. I'm claiming that it is surprising. Users are being taught (via experience, and via the fact that we're telling them so) that
I'm not even really sure - Dart's grammar is weird here. I don't know how to think of |
|
@tatumizer Are you assuming that In any case, that is a syntactically valid expression. Whether it has errors and/or warnings depends somewhat on the type of |
|
Assuming that If
if
Note that what you actually wanted to write, given your description of your intended semantics, is the same in either case, and does not involve |
There is no proposal that the compiler reject this. In either semantics, the second If |
I'm not sure I agree that we're telling them that, at least not that alone. I do admit that my fixation on how things bind might be a little too syntax based. The question is not how to parse (Does a cascade terminate null shortening? I still think letting I can't write I think that users who understand null-shortening So, I think the safest move for now is to prohibit trailing |
From https://dart.dev/null-safety/understanding-null-safety This is public documentation written by a member of the language team. A recent internal email to Dart users with a TL;DR about null safety used basically the same description. We can certainly change what we are telling people, but it's definitely what we're telling them.
I really think this is an odd thing to prioritize. Even putting aside "except for the result of the entire expression", which is doing rather a lot of work here,
Why cascades? |
|
Cascades for consistency. I still think users have a benefit from thinking of "downstream from |
I may have misunderstood you, do you mean one these, or something else?
|
|
I think I'd go for:
So, the first of those four. |
I'm only aware of one instance in which this arose concretely: when Michal Terepeta was trying to migrate pageloader (see internal bug 162579669). He tried to change |
|
For the record, Another possible postfix operator is the tightly-binding-cast, Should Since (I say yes, because I want all selectors to participate, then the reach of the |
|
Putting aside the cascades and the If we do agree that it should participate then we'd at least cut down on this discussion to be about the terminating I tend to think that it would be highly confusing if return hasActiveBudget
? row?.insertionOrder.activeBudget.budgetType
: row?.insertionOrder.totalBudget!.budgetType;Assuming that the getters We do seem to have support for making the non-terminating |
|
I am currently assuming that non-terminating I've been talking about how I think cascades, null-shortening and general selector chains should behave similarly. Talking to Leaf yesterday made me able to explain (I hope) why I think it's important. At least I'll try. I believe that users think of programs (among other ways) by understanding some concepts by mentally desugaring them to simpler concepts, and by knowing some "safe transformations" (which is probably just a word for refactoring). Cascade to non-cascadeA cascade, let tmp = e in {tmp.selectors1; tmp.selectors2; return tmp;}(A fancy code-like way to say: Evaluate e to a value tmp, then execute the selector chain selectors1 on v, then do the same for selectors2, finally the value of the entire cascade is v. If I expect users to perform something like this desugaring in their head, in order to figure out what a cascade actually does. This desugars cascade That is, users do not expect cascade section selectors to be a special thing, they are understood exactly like the same non-cascade selectors. For this reasoning to be correct, the meaning of the selectors must not change when they are moved from inside a cascade to not being inside a cascade. We even have a lint which makes users change Null-aware cascade to plain cascadeA null-aware cascade, let tmp = e in tmp == null ? null : tmp..selectors1..selectors2which can then be further desugared to the non-cascade like above, without even needing an extra variable. For this reasoning to be correct, the meaning of selectors must not change when moved from a null-aware cascade to a non-null-aware cascade. (A null-aware cascade can also be desugared as to normal null-aware selectors: let tmp = e in {tmp?.selectors1; tmp?.selectors2; tmp}Either works, it all depends on whether the user thinks of In general, the cascade sections should act the same whether it's a Null-aware cascade to null-aware selectorThe unnecessary single cascade lint will likely also recommend changing Non-cascade to cascade.If you write: something.selectors1;
something.selectors2;then we recommend combining them into a cascade: something
..selectors1
..selectors2;For that to be correct, the meaning of a selector chain must not change when moved from a non-cascade to a cascade. (We'll probably also recommend Conditional null-aware to null-checking selectorUsers will think of If you wanted to write
That means that if you are looking at For this reasoning to be correct, it's important that the meaning of Consequences for
|
|
I just noticed that https://github.com/dart-lang/language/blob/master/accepted/future-releases/nnbd/feature-specification.md says:
That's not a sufficient syntactic specification. The linked grammar makes it a selector, which makes it clear how it parses. Maybe the phrasing should be changed to say something similar to what we say for |
It's not intended to be. That's why there's a grammar. |
I don't understand what you are trying to say here. Or rather, I don't think you're saying what you mean to say. Under the assumption that e does not evaluate to
I think what you might be trying to say is that |
I don't have a lot to say about this. I understand broadly where you're coming from, but I don't find it all convincing to drive the design of a major part of the language (
This is a completely invalid refactoring, and I don't believe that we recommend it.... unless |
|
My basic view on the situation is as follows. The situation as it stands We have two constructs Currently Currently, The proposed change The proposal is to change this. Under this proposal, Simple re-factors like replacing On the other hand, Relative importance So we have two opposing desires: for various reasons, there's a desire to make a change that improves the behavior of On the other hand, Definability Note that if you really want a null shorting bang operator, you can define it already: extension NullCheck<T> on T? {
T get notNull => this!;
}
void main() {
List<int>? l = null;
print(l?[0].notNull.isEven); // Prints "null"
} |
For the record, yes... I know. |
Note that if you look at this code, it's quite clear that the expectation in the original migration was that |
Yes. |
Yes, we only recommend it when it doesn't change semantics in a way that affects the correctness of the program. It's not in the style guide, but it's a lint, and I've seen plenty of code reviews where someone is asked to change The rewrite would not be valid if |
|
While The failure modes of the two are different.
Being non-participating is more likely to introduce accidental errors. |
|
I tried looking at what other languages do. Not a lot of conclusion from that, though.
|
OK, let me try to say this differently. I'm looking at the question of "What choice should we the language designers make here?" I'm trying to decide what behavior I consider it "useful" for us to put into the language. Maybe "worthwhile" is a better term. So, trying to evaluate which behavior is worthwhile for us to put into the language:
No, I think you're exactly backwards here. Perl's motto literally is "there's more than one way to do it". If we don't treat "you can already do that easily in Dart" as a reason to not add something, then it will encourage us to add more redundant ways to express the same thing, which is how you get Perl. I don't want Perl either. (Though I admit UI-as-code certainly gives you extra redundant ways to make lists...) Taking your quote, what I propose is extending it like, "sure writing this piece of syntax looks like it should do Y, but users already have better a way to write Y, so we're going to define it to do X which they don't have a way to do yet and will want". Why give them two ways to express Y (one of which is kind of bad) and no ways to express X? |
I'm sorry, but I am completely unconvinced by this line of reasoning. We have many, many, many constructs in Dart that are redundant with other constructs. The argument that
My point, which I have been making very consistently, is that this may be necessary, but it is not sufficient. I agree with the above: making null-shorting participate is a more useful choice. However, this is not sufficient to make me think that this is the right choice, because I believe that making such a choice makes the language harder to read, and harder to write. And so I believe that it is better not to provide this convenient shortcut. Users who want this behavior will have to write their code differently. The result will be less convenient to write, but much more likely to be correctly interpreted by a reader. |
This seems very stretched to me. You're saying "I have some code. It can clearly evaluate to |
|
I do agree with Leaf that we should design the behavior based on what is most intuitive to the average reader, and not what is the most expressive/useful to a sufficiently informed writer. What I don't agree about is that the non-null shortening behavior is intuitive. The little evidence we do have actually suggests the opposite if anything. Ultimately, all the evidence that I have seen so far tells me that both We already know that it is a rare scenario anyways, and you can use parenthesis to get the non-null shortening behavior, which while a bit more verbose is very clear as to the intention. The null shortening behavior is more verbose, but it will also be very explicit. Making it an error also buys us the time to do proper user studies in the future, if desired, to make sure we get the behavior right. Or we just leave it as an error if nobody complains. |
I really hate to keep harping on this, but this is simply not true. We have three solid examples of people encountering a null shorting expression ending in It is also true that a small majority of users that we've asked about this think that This is why I keep distinguishing between I am therefore, at this point as comfortable as it is possible to be given the limited evidence in saying the following:
I understand that those two data points lead to opposite conclusions. |
But in the informal survey 60% of respondents preferred the opposite. Yes, that survey does not present the two cases in isolation, but it does have a lot more respondents and from more typical users (and I also don't think it is a bad thing necessarily for them to have to consider both cases when forming an opinion). |
I did address this in my comment. The question of whether a user, when prompted to think about the semantics of I did mention this when the survey was being written: it would be a much more effective survey if users were not presented the questions together, but rather were randomly assigned to either be prompted first or not, with no opportunity to change their previous answers. Such a survey would give some insight into how people think about |
Nobody is going to be exposed to only one of those. Users won't have to intuit the meaning of expressions from scratch each time, they will learn about both, and how they read things then is more important than first impressions. And I believe a null-shortening participating |
|
We are getting closer to a deadline, but no closer to agreement, so I propose that we:
A trailing or a (no assignment after), where the last selector matched by (an Making a This postpones the decision about what to do. It's still possible to do a non-participating trailing extension NonNull<T extends Object> on T? {
T get nonNull => this as T;
}We won't be locked into either behavior prior to being able to decide which one we want. This proposal is based on the assumption that it's easier to introduce an error than it is to change existing behavior, and therefore more likely that we can do so in time. |
|
Decision: We'll ask the implementors which approach is most expedient:
|
Bug: dart-lang/language#1163 Change-Id: I6e648f76413c5036a867b2125359f28eb03027c2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/163727 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]>
|
I believe this is done. |
This issue captures discusison that has been happening in dart-lang/sdk#43093.
Quoting @lrhn:
Quoting @eernstg:
The text was updated successfully, but these errors were encountered: