Skip to content

Commit 7b6c667

Browse files
stereotype441commit-bot@chromium.org
stereotype441
authored and
[email protected]
committed
Flow analysis: rework tracking of nullability.
We now track nullability via type promotion, e.g. by promoting from `int?` to `int` to represent a non-nullable value. I've added a few more unit tests of flow analysis that were helpful while preparing this CL. I plan to add more tests in follow-up CLs. A few behaviors are lost by this change: - We no longer track whether a variable's value is exactly null (previously the tests annotated such variables as "nullable", which was a bit misleading). I'm not sure whether we really want this feature or not; I've filed dart-lang/language#461 for discussion. - Assignment of a non-null value to a nullable-typed variable no longer promotes it to non-nullable. I think we want to get this behavior back, but we'll need to add a little bit more machinery to get it back (we need to allow assignments in general to promote. I'll address in follow-up CLs. See dart-lang/language#440 for discussion. - A promotion to non-nullable that happens during a "try" block is lost if there is a "finally" block. I think we want to get this behavior back, but we'll need to add a little bit more machinery to do so, to allow promotions in general to be preserved by try/finally. I'll address in follow-up CLs. Change-Id: Ibc45d70043725697fbee063a8dcefb1179506e9b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109780 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
1 parent c2e78e8 commit 7b6c667

File tree

9 files changed

+194
-209
lines changed

9 files changed

+194
-209
lines changed

‎pkg/analyzer/lib/dart/element/type_system.dart‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,10 @@ abstract class TypeSystem {
125125
/// Other type systems may define this operation differently.
126126
DartType leastUpperBound(DartType leftType, DartType rightType);
127127

128+
/// Returns a non-nullable version of [type]. This is equivalent to the
129+
/// operation `NonNull` defined in the spec.
130+
DartType promoteToNonNull(DartType type);
131+
128132
/// Return the result of resolving the bounds of the given [type].
129133
///
130134
/// For the Dart 2.0 type system, the definition of resolving to bounds is

‎pkg/analyzer/lib/src/dart/resolver/flow_analysis.dart‎

Lines changed: 34 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,6 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
8888
var identifyState = _State<Variable, Type>(
8989
false,
9090
emptySet,
91-
emptySet,
92-
emptySet,
9391
const {},
9492
);
9593
return FlowAnalysis._(
@@ -111,8 +109,6 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
111109
_current = _State<Variable, Type>(
112110
true,
113111
_emptySet,
114-
_emptySet,
115-
_emptySet,
116112
const {},
117113
);
118114
}
@@ -193,8 +189,9 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
193189
}
194190

195191
_condition = binaryExpression;
196-
_conditionTrue = _current.markNullable(_emptySet, variable);
197-
_conditionFalse = _current.markNonNullable(_emptySet, variable);
192+
_conditionTrue = _current;
193+
_conditionFalse =
194+
_current.markNonNullable(typeOperations, _emptySet, variable);
198195
}
199196

200197
/// The [binaryExpression] checks that the [variable] is not equal to `null`.
@@ -204,8 +201,9 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
204201
}
205202

206203
_condition = binaryExpression;
207-
_conditionTrue = _current.markNonNullable(_emptySet, variable);
208-
_conditionFalse = _current.markNullable(_emptySet, variable);
204+
_conditionTrue =
205+
_current.markNonNullable(typeOperations, _emptySet, variable);
206+
_conditionFalse = _current;
209207
}
210208

211209
void doStatement_bodyBegin(
@@ -379,16 +377,6 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
379377
}
380378
}
381379

382-
/// Return `true` if the [variable] is known to be be non-nullable.
383-
bool isNonNullable(Variable variable) {
384-
return !_current.notNonNullable.contains(variable);
385-
}
386-
387-
/// Return `true` if the [variable] is known to be be nullable.
388-
bool isNullable(Variable variable) {
389-
return !_current.notNullable.contains(variable);
390-
}
391-
392380
@visibleForTesting
393381
Map<Variable, Type> joinPromoted(
394382
Map<Variable, Type> first,
@@ -610,13 +598,8 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
610598
}
611599

612600
/// Register write of the given [variable] in the current state.
613-
void write(
614-
Variable variable, {
615-
bool isNull = false,
616-
bool isNonNull = false,
617-
}) {
618-
_current = _current.write(typeOperations, _emptySet, variable,
619-
isNull: isNull, isNonNull: isNonNull);
601+
void write(Variable variable) {
602+
_current = _current.write(typeOperations, _emptySet, variable);
620603
}
621604

622605
void _conditionalEnd(Expression condition) {
@@ -642,17 +625,13 @@ class FlowAnalysis<Statement, Expression, Variable, Type> {
642625

643626
var newReachable = first.reachable || second.reachable;
644627
var newNotAssigned = first.notAssigned.union(second.notAssigned);
645-
var newNotNullable = first.notNullable.union(second.notNullable);
646-
var newNotNonNullable = first.notNonNullable.union(second.notNonNullable);
647628
var newPromoted = joinPromoted(first.promoted, second.promoted);
648629

649630
return _State._identicalOrNew(
650631
first,
651632
second,
652633
newReachable,
653634
newNotAssigned,
654-
newNotNullable,
655-
newNotNonNullable,
656635
newPromoted,
657636
);
658637
}
@@ -679,42 +658,39 @@ abstract class TypeOperations<Variable, Type> {
679658
/// Return `true` if the [leftType] is a subtype of the [rightType].
680659
bool isSubtypeOf(Type leftType, Type rightType);
681660

661+
/// Returns the non-null promoted version of [type], if it is different,
662+
/// otherwise `null`. For example, given `int?`, returns `int`.
663+
///
664+
/// Note that some types don't have a non-nullable version (e.g.
665+
/// `FutureOr<int?>`), so `null` may be returned even if the type is nullable.
666+
Type tryPromoteToNonNull(Type type);
667+
682668
/// Return the static type of the given [variable].
683669
Type variableType(Variable variable);
684670
}
685671

686672
class _State<Variable, Type> {
687673
final bool reachable;
688674
final _VariableSet<Variable> notAssigned;
689-
final _VariableSet<Variable> notNullable;
690-
final _VariableSet<Variable> notNonNullable;
691675
final Map<Variable, Type> promoted;
692676

693677
_State(
694678
this.reachable,
695679
this.notAssigned,
696-
this.notNullable,
697-
this.notNonNullable,
698680
this.promoted,
699681
);
700682

701683
/// Add a new [variable] to track definite assignment.
702684
_State<Variable, Type> add(Variable variable, {bool assigned: false}) {
703685
var newNotAssigned = assigned ? notAssigned : notAssigned.add(variable);
704-
var newNotNullable = notNullable.add(variable);
705-
var newNotNonNullable = notNonNullable.add(variable);
706686

707-
if (identical(newNotAssigned, notAssigned) &&
708-
identical(newNotNullable, notNullable) &&
709-
identical(newNotNonNullable, notNonNullable)) {
687+
if (identical(newNotAssigned, notAssigned)) {
710688
return this;
711689
}
712690

713691
return _State<Variable, Type>(
714692
reachable,
715693
newNotAssigned,
716-
newNotNullable,
717-
newNotNonNullable,
718694
promoted,
719695
);
720696
}
@@ -723,48 +699,29 @@ class _State<Variable, Type> {
723699
return _State<Variable, Type>(
724700
false,
725701
notAssigned,
726-
notNullable,
727-
notNonNullable,
728702
promoted,
729703
);
730704
}
731705

732706
_State<Variable, Type> markNonNullable(
733-
_VariableSet<Variable> emptySet, Variable variable) {
734-
var newNotNullable = notNullable.add(variable);
735-
var newNotNonNullable = notNonNullable.remove(emptySet, variable);
736-
737-
if (identical(newNotNullable, notNullable) &&
738-
identical(newNotNonNullable, notNonNullable)) {
739-
return this;
740-
}
741-
742-
return _State<Variable, Type>(
743-
reachable,
744-
notAssigned,
745-
newNotNullable,
746-
newNotNonNullable,
747-
promoted,
748-
);
749-
}
750-
751-
_State<Variable, Type> markNullable(
752-
_VariableSet<Variable> emptySet, Variable variable) {
753-
var newNotNullable = notNullable.remove(emptySet, variable);
754-
var newNotNonNullable = notNonNullable.add(variable);
707+
TypeOperations<Variable, Type> typeOperations,
708+
_VariableSet<Variable> emptySet,
709+
Variable variable) {
710+
var previousType = promoted[variable];
711+
previousType ??= typeOperations.variableType(variable);
712+
var type = typeOperations.tryPromoteToNonNull(previousType);
755713

756-
if (identical(newNotNullable, notNullable) &&
757-
identical(newNotNonNullable, notNonNullable)) {
758-
return this;
714+
if (type != null) {
715+
var newPromoted = <Variable, Type>{}..addAll(promoted);
716+
newPromoted[variable] = type;
717+
return _State<Variable, Type>(
718+
reachable,
719+
notAssigned,
720+
newPromoted,
721+
);
759722
}
760723

761-
return _State<Variable, Type>(
762-
reachable,
763-
notAssigned,
764-
newNotNullable,
765-
newNotNonNullable,
766-
promoted,
767-
);
724+
return this;
768725
}
769726

770727
_State<Variable, Type> promote(
@@ -782,8 +739,6 @@ class _State<Variable, Type> {
782739
return _State<Variable, Type>(
783740
reachable,
784741
notAssigned,
785-
notNullable,
786-
notNonNullable,
787742
newPromoted,
788743
);
789744
}
@@ -792,19 +747,13 @@ class _State<Variable, Type> {
792747
}
793748

794749
_State<Variable, Type> removePromotedAll(Set<Variable> variables) {
795-
var newNotNullable = notNullable.addAll(variables);
796-
var newNotNonNullable = notNonNullable.addAll(variables);
797750
var newPromoted = _removePromotedAll(promoted, variables);
798751

799-
if (identical(newNotNullable, notNullable) &&
800-
identical(newNotNonNullable, notNonNullable) &&
801-
identical(newPromoted, promoted)) return this;
752+
if (identical(newPromoted, promoted)) return this;
802753

803754
return _State<Variable, Type>(
804755
reachable,
805756
notAssigned,
806-
newNotNullable,
807-
newNotNonNullable,
808757
newPromoted,
809758
);
810759
}
@@ -821,21 +770,6 @@ class _State<Variable, Type> {
821770
other: other.notAssigned,
822771
);
823772

824-
var newNotNullable = emptySet;
825-
for (var variable in notNullable.variables) {
826-
if (unsafe.contains(variable) || other.notNullable.contains(variable)) {
827-
newNotNullable = newNotNullable.add(variable);
828-
}
829-
}
830-
831-
var newNotNonNullable = emptySet;
832-
for (var variable in notNonNullable.variables) {
833-
if (unsafe.contains(variable) ||
834-
other.notNonNullable.contains(variable)) {
835-
newNotNonNullable = newNotNonNullable.add(variable);
836-
}
837-
}
838-
839773
var newPromoted = <Variable, Type>{};
840774
for (var variable in promoted.keys) {
841775
var thisType = promoted[variable];
@@ -855,8 +789,6 @@ class _State<Variable, Type> {
855789
other,
856790
newReachable,
857791
newNotAssigned,
858-
newNotNullable,
859-
newNotNonNullable,
860792
newPromoted,
861793
);
862794
}
@@ -867,45 +799,26 @@ class _State<Variable, Type> {
867799
return _State<Variable, Type>(
868800
reachable,
869801
notAssigned,
870-
notNullable,
871-
notNonNullable,
872802
promoted,
873803
);
874804
}
875805

876-
_State<Variable, Type> write(
877-
TypeOperations<Variable, Type> typeOperations,
878-
_VariableSet<Variable> emptySet,
879-
Variable variable, {
880-
bool isNull = false,
881-
bool isNonNull = false,
882-
}) {
806+
_State<Variable, Type> write(TypeOperations<Variable, Type> typeOperations,
807+
_VariableSet<Variable> emptySet, Variable variable) {
883808
var newNotAssigned = typeOperations.isLocalVariable(variable)
884809
? notAssigned.remove(emptySet, variable)
885810
: notAssigned;
886811

887-
var newNotNullable = isNull
888-
? notNullable.remove(emptySet, variable)
889-
: notNullable.add(variable);
890-
891-
var newNotNonNullable = isNonNull
892-
? notNonNullable.remove(emptySet, variable)
893-
: notNonNullable.add(variable);
894-
895812
var newPromoted = _removePromoted(promoted, variable);
896813

897814
if (identical(newNotAssigned, notAssigned) &&
898-
identical(newNotNullable, notNullable) &&
899-
identical(newNotNonNullable, notNonNullable) &&
900815
identical(newPromoted, promoted)) {
901816
return this;
902817
}
903818

904819
return _State<Variable, Type>(
905820
reachable,
906821
newNotAssigned,
907-
newNotNullable,
908-
newNotNonNullable,
909822
newPromoted,
910823
);
911824
}
@@ -952,30 +865,22 @@ class _State<Variable, Type> {
952865
_State<Variable, Type> second,
953866
bool newReachable,
954867
_VariableSet<Variable> newNotAssigned,
955-
_VariableSet<Variable> newNotNullable,
956-
_VariableSet<Variable> newNotNonNullable,
957868
Map<Variable, Type> newPromoted,
958869
) {
959870
if (first.reachable == newReachable &&
960871
identical(first.notAssigned, newNotAssigned) &&
961-
identical(first.notNullable, newNotNullable) &&
962-
identical(first.notNonNullable, newNotNonNullable) &&
963872
identical(first.promoted, newPromoted)) {
964873
return first;
965874
}
966875
if (second.reachable == newReachable &&
967876
identical(second.notAssigned, newNotAssigned) &&
968-
identical(second.notNullable, newNotNullable) &&
969-
identical(second.notNonNullable, newNotNonNullable) &&
970877
identical(second.promoted, newPromoted)) {
971878
return second;
972879
}
973880

974881
return _State<Variable, Type>(
975882
newReachable,
976883
newNotAssigned,
977-
newNotNullable,
978-
newNotNonNullable,
979884
newPromoted,
980885
);
981886
}

0 commit comments

Comments
 (0)