Open
Description
void function(int? value) {
Null nil;
if (value == null) {
nil = value; // A value of type 'int?' can't be assigned to a variable of type 'Null'. Try changing the type of the variable, or casting the right-hand type to 'Null'.
nil = null; // works
}
}
I was surprised that this isn't allowed. Is this a bug or is this expected behavior?
Metadata
Metadata
Assignees
Labels
Type
Projects
Milestone
Relationships
Development
No branches or pull requests
Activity
leafpetersen commentedon Mar 10, 2021
This was discussed here and the resolution was that we intended to promote. @stereotype441 looks like this is a bug, or did we change our mind on this? Not sure whether we can fix this now, though it's pretty unlikely to be de facto breaking.
stereotype441 commentedon Mar 22, 2021
Yeah, it looks like this was an oversight. I'll try to find some time this week to investigate whether it would cause any internal breakages to change this behavior; that should be a pretty good proxy for whether external users would be affected.
stereotype441 commentedon Mar 22, 2021
@leafpetersen
Unfortunately, it seems to be quite breaking after all. So far I haven't even gotten the Dart SDK to build with this change. There's a lot of failures and I haven't checked them all out (you can look at https://dart-review.googlesource.com/c/sdk/+/192420 if you want to see details), but I've seen enough that I don't think we should consider changing this now, right after a stable release.
FWIW, here are the two coding patterns I've found so far that would be broken by this change:
Dead code
If a function contains dead code for handling non-null values, the promotion can carry into it, causing type errors, e.g.:
Maybe we could come up with a clever way to modify flow analysis to fix this. For example, maybe at the time we detect unreachability, we un-do the promotion to
Null
, so the type ofi
goes back toint?
and then gets promoted toint
.Type inference when setting a null value to non-null
This type of construction appears to be pretty common:
What's happening is that when analyzing an assignment to a variable, we use the promoted type of the LHS as the context for the RHS, so that we only demote when that context can't be satisfied. But since the promoted type of the LHS is now
Null
rather thanList<int>?
, we've lost the information about what type of list element we want.Again, some cleverness could probably address this. We could, for instance, say that when analyzing an assignment, we use the promoted type of the LHS only if it's not
Null
; if it'sNull
, we pop one step up in the promotion chain and use the previous promoted type (or the declared type, if there was no previous promotion).== null
to promote toNull
#1959lrhn commentedon Nov 6, 2021
In the
example, is there any way we can promote
i
toNever
in the second block?We've promoted it to
Null
before (akaNever?
), and now we check that it's also notNull
, which could potentially promote it toNever
proper.Not sure how useful it is. The code is dead, so all we can say is that we never have a value for
i
reaching there.leafpetersen commentedon Nov 13, 2021
This would be the natural outcome of using the factor algorithm as specified in the flow analysis design doc. I'm not sure we do that though.
stereotype441 commentedon Nov 22, 2021
I prototyped a change to the type system that allows promotion to
Null
, and I found that there are some unfortunate consequences.The biggest effect is on code like this (adapted from
package:dart_style/src/rule/rule.dart
):At the line marked (1), under the current rules, where we don't promote to
Null
, the literal{}
is type inferred with the context typeSet<Rule>
, so it's interpreted as an empty set literal (rather than an empty map literal). But if we allow promotion toNull
, then the literal{}
is type inferred with the context typeNull
, so there is no contextual information to tell whether it should be a map or a set literal. We default to assuming it's a map literal, and that results in a compile-time error.The problem doesn't just affect the set/map distinction; it can happen to lists too. Consider this example (from
package:sass/src/extend/extension_store.dart
):In this case, at the line marked (1), there's question that
[]
represents a list, but the type promotion toNull
takes away the necessary context to see what kind of a list it should be, so type inference infers it as aList<dynamic>
, which is not assignable toList<List<Extender>>?
.I found several dozen examples of these two patterns.
Another pattern, which is less common, but still problematic, occurs when promotion to
Null
causes dead code to fail to type check. This example is frompackage:typed_data/src/typed_buffer.dart
:I've elided a lot of code here, so I should mention that there are no assignments to
end
anywhere in this method. Therefore, theif
condition at (2) will always befalse
(because the case ofend == null
was handled at (1)), and in point of fact, the&& writeIndex < end
part of the condition is redundant. Ideally, the analyzer's dead code hint should warn about the fact that it's redundant, but it doesn't, because we don't currently allow promotion toNull
.If we do start allowing promotion to
Null
, here's what happens: at (2), the type ofend
isNull
, so the expressionwriteIndex < end
now has a type error, and the code fails to compile.It's my personal belief that these problems outweigh the benefit of allowing promotion to
Null
, so we should leave the language as it is and not allow== null
checks to promote toNull
.lrhn commentedon Nov 23, 2021
I'm assuming the
rules
declaration should be nullableThe problem seems to be that we assume that the context type of the assignment to a promoted variable can always be represented by the most promoted type, because if any type in the promotion chain has an instantiation of an interface type, all of them must agree. That's not true for
Null
... orNever
! It's already an issue if you do explicit promotion toNull
.And, since we allow demoting assignments, the currently promoted type is not the only relevant type for assignment.
I can see that we don't want to do type inference on the sub-expression more than once.
What if a context type could an entire chain of promoted types. Most of the time, it's just one type, but for assignment to promoted variables (and propagated down through that expression), every one of the types is seen as a constraint. They'll mostly agree, or be specializations, but in a few cases, where you promote a union type to one of its branches, there might be a constraint further up the chain. That goes for
FutureOr
too:I don't know how to actually use a chain of types as context type. I guess if you need a type argument to a generic type, you check whether any of the types in the chain implements that generic type, and then you still have to choose one of those type arguments, likely the most specific. (Union types as context types are just not very useful - another reason I don't want union types in the language.) For every other use of context types, you'd probably just use the most specific promoted type.
The:
again looks like something where the second
end
should have been promoted toNever
since doingend != null
on something typedNull
, akaNever?
, should promote it toNever
.Then the analyzer should warn that the code is dead (because it can contain any number of incorrect uses of
end
without any further errors.)==
patterns to have a receiver pattern? #2620[dart2js] Don't rely on `== null` promoting to Null in rti.dart.
??
insideif null
test when the body does more things dart-lang/sdk#59555Object?
toNull
dart-lang/sdk#60741