Create a data structure to track the graph of nullability nodes.
The previous design (which stored the graph impicitly in the nodes
themselves) had a memory leak because some nullability nodes are
static, meaning that once a node became connected to one of the static
nodes, it would never be garbage collected.
Change-Id: I9821660198e29ff974ee0afa678851e0418e0527
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/101487
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
diff --git a/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart b/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
index 52ed8a5..c6d9e80 100644
--- a/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
+++ b/pkg/analysis_server/lib/src/nullability/constraint_gatherer.dart
@@ -6,6 +6,7 @@
import 'package:analysis_server/src/nullability/constraint_variable_gatherer.dart';
import 'package:analysis_server/src/nullability/decorated_type.dart';
import 'package:analysis_server/src/nullability/expression_checks.dart';
+import 'package:analysis_server/src/nullability/nullability_graph.dart';
import 'package:analysis_server/src/nullability/nullability_node.dart';
import 'package:analysis_server/src/nullability/transitional_api.dart';
import 'package:analysis_server/src/nullability/unit_propagation.dart';
@@ -38,6 +39,8 @@
/// Constraints gathered by the visitor are stored here.
final Constraints _constraints;
+ final NullabilityGraph _graph;
+
/// The file being analyzed.
final Source _source;
@@ -78,8 +81,14 @@
/// or expression.
bool _inConditionalControlFlow = false;
- ConstraintGatherer(TypeProvider typeProvider, this._variables,
- this._constraints, this._source, this._permissive, this.assumptions)
+ ConstraintGatherer(
+ TypeProvider typeProvider,
+ this._variables,
+ this._constraints,
+ this._graph,
+ this._source,
+ this._permissive,
+ this.assumptions)
: _notNullType =
DecoratedType(typeProvider.objectType, NullabilityNode.never),
_nonNullableBoolType =
@@ -207,7 +216,7 @@
var overallType = DecoratedType(
node.staticType,
NullabilityNode.forLUB(
- node, thenType.node, elseType.node, _joinNullabilities));
+ node, thenType.node, elseType.node, _graph, _joinNullabilities));
_variables.recordDecoratedExpressionType(node, overallType);
return overallType;
}
@@ -228,6 +237,7 @@
null,
_guards,
_constraints,
+ _graph,
false);
} else {
assert(assumptions.namedNoDefaultParameterHeuristic ==
@@ -438,7 +448,7 @@
expression.end, sourceType.node, destinationType.node, _guards));
}
NullabilityNode.recordAssignment(sourceType.node, destinationType.node,
- checkNotNull, _guards, _constraints, _inConditionalControlFlow);
+ checkNotNull, _guards, _constraints, _graph, _inConditionalControlFlow);
// TODO(paulberry): it's a cheat to pass in expression=null for the
// recursive checks. Really we want to unify all the checks in a single
// ExpressionChecks object.
diff --git a/pkg/analysis_server/lib/src/nullability/nullability_graph.dart b/pkg/analysis_server/lib/src/nullability/nullability_graph.dart
new file mode 100644
index 0000000..f1c9d45
--- /dev/null
+++ b/pkg/analysis_server/lib/src/nullability/nullability_graph.dart
@@ -0,0 +1,28 @@
+// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analysis_server/src/nullability/nullability_node.dart';
+
+/// Data structure to keep track of the relationship between [NullabilityNode]
+/// objects.
+class NullabilityGraph {
+ /// Map from a nullability node to those nodes that are "downstream" from it
+ /// (meaning that if a key node is nullable, then all the nodes in the
+ /// corresponding value will either have to be nullable, or null checks will
+ /// have to be added).
+ final _downstream = Map<NullabilityNode, List<NullabilityNode>>.identity();
+
+ /// Records that [sourceNode] is immediately upstream from [destinationNode].
+ void connect(NullabilityNode sourceNode, NullabilityNode destinationNode) {
+ (_downstream[sourceNode] ??= []).add(destinationNode);
+ }
+
+ /// Iterates through all nodes that are "downstream" of [node] (i.e. if
+ /// [node] is nullable, then all the iterated nodes will either have to be
+ /// nullable, or null checks will have to be added).
+ ///
+ /// There is no guarantee of uniqueness of the iterated nodes.
+ Iterable<NullabilityNode> getDownstreamNodes(NullabilityNode node) =>
+ _downstream[node] ?? const [];
+}
diff --git a/pkg/analysis_server/lib/src/nullability/nullability_node.dart b/pkg/analysis_server/lib/src/nullability/nullability_node.dart
index e244d0e..e871c28 100644
--- a/pkg/analysis_server/lib/src/nullability/nullability_node.dart
+++ b/pkg/analysis_server/lib/src/nullability/nullability_node.dart
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:analysis_server/src/nullability/decorated_type.dart';
+import 'package:analysis_server/src/nullability/nullability_graph.dart';
import 'package:analysis_server/src/nullability/transitional_api.dart';
import 'package:analysis_server/src/nullability/unit_propagation.dart';
import 'package:analyzer/dart/ast/ast.dart';
@@ -30,11 +31,6 @@
/// migrated) forces this type to be non-nullable.
final ConstraintVariable nullable;
- /// List of all the nodes that are "downstream" of this one (i.e. if this node
- /// is nullable, then all the nodes in [_downstreamNodes] will either have to
- /// be nullable, or null checks will have to be added).
- final _downstreamNodes = <NullabilityNode>[];
-
ConstraintVariable _nonNullIntent;
bool _isPossiblyOptional = false;
@@ -57,6 +53,7 @@
Expression conditionalExpression,
NullabilityNode a,
NullabilityNode b,
+ NullabilityGraph graph,
ConstraintVariable Function(
Expression, ConstraintVariable, ConstraintVariable)
joinNullabilities) = NullabilityNodeForLUB._;
@@ -88,11 +85,6 @@
/// annotate the nullability of that type.
String get debugSuffix => nullable == null ? '' : '?($nullable)';
- /// Iterates through all nodes that are "downstream" of this node (i.e. if
- /// this node is nullable, then all the nodes in [downstreamNodes] will either
- /// have to be nullable, or null checks will have to be added).
- Iterable<NullabilityNode> get downstreamNodes => _downstreamNodes;
-
/// Indicates whether this node is always nullable, by construction.
bool get isAlwaysNullable => identical(nullable, ConstraintVariable.always);
@@ -162,9 +154,10 @@
CheckExpression checkNotNull,
List<NullabilityNode> guards,
Constraints constraints,
+ NullabilityGraph graph,
bool inConditionalControlFlow) {
var additionalConditions = <ConstraintVariable>[];
- sourceNode._downstreamNodes.add(destinationNode);
+ graph.connect(sourceNode, destinationNode);
if (sourceNode.nullable != null) {
additionalConditions.add(sourceNode.nullable);
var destinationNonNullIntent = destinationNode.nonNullIntent;
@@ -223,12 +216,13 @@
Expression expression,
this.left,
this.right,
+ NullabilityGraph graph,
ConstraintVariable Function(
ConditionalExpression, ConstraintVariable, ConstraintVariable)
joinNullabilities)
: super._(joinNullabilities(expression, left.nullable, right.nullable)) {
- left._downstreamNodes.add(this);
- right._downstreamNodes.add(this);
+ graph.connect(left, this);
+ graph.connect(right, this);
}
}
diff --git a/pkg/analysis_server/lib/src/nullability/transitional_api.dart b/pkg/analysis_server/lib/src/nullability/transitional_api.dart
index 7a1b505..ac1d8df 100644
--- a/pkg/analysis_server/lib/src/nullability/transitional_api.dart
+++ b/pkg/analysis_server/lib/src/nullability/transitional_api.dart
@@ -7,6 +7,7 @@
import 'package:analysis_server/src/nullability/constraint_variable_gatherer.dart';
import 'package:analysis_server/src/nullability/decorated_type.dart';
import 'package:analysis_server/src/nullability/expression_checks.dart';
+import 'package:analysis_server/src/nullability/nullability_graph.dart';
import 'package:analysis_server/src/nullability/nullability_node.dart';
import 'package:analysis_server/src/nullability/unit_propagation.dart';
import 'package:analyzer/dart/ast/ast.dart';
@@ -149,6 +150,8 @@
final _constraints = Solver();
+ final _graph = NullabilityGraph();
+
/// Prepares to perform nullability migration.
///
/// If [permissive] is `true`, exception handling logic will try to proceed
@@ -172,7 +175,7 @@
void processInput(CompilationUnit unit, TypeProvider typeProvider) {
unit.accept(ConstraintGatherer(typeProvider, _variables, _constraints,
- unit.declaredElement.source, _permissive, assumptions));
+ _graph, unit.declaredElement.source, _permissive, assumptions));
}
}
diff --git a/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart b/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
index cc45432..5501916 100644
--- a/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
+++ b/pkg/analysis_server/test/src/nullability/migration_visitor_test.dart
@@ -7,6 +7,7 @@
import 'package:analysis_server/src/nullability/constraint_variable_gatherer.dart';
import 'package:analysis_server/src/nullability/decorated_type.dart';
import 'package:analysis_server/src/nullability/expression_checks.dart';
+import 'package:analysis_server/src/nullability/nullability_graph.dart';
import 'package:analysis_server/src/nullability/nullability_node.dart';
import 'package:analysis_server/src/nullability/transitional_api.dart';
import 'package:analysis_server/src/nullability/unit_propagation.dart';
@@ -31,6 +32,9 @@
@override
final _Constraints constraints = _Constraints();
+ @override
+ final graph = NullabilityGraph();
+
void assertConditional(
NullabilityNode node, NullabilityNode left, NullabilityNode right) {
var conditionalNode = node as NullabilityNodeForLUB;
@@ -788,6 +792,8 @@
abstract class ConstraintsTestBase extends MigrationVisitorTestBase {
Constraints get constraints;
+ NullabilityGraph get graph;
+
/// Analyzes the given source code, producing constraint variables and
/// constraints for it.
@override
@@ -795,8 +801,8 @@
{NullabilityMigrationAssumptions assumptions:
const NullabilityMigrationAssumptions()}) async {
var unit = await super.analyze(code);
- unit.accept(ConstraintGatherer(
- typeProvider, _variables, constraints, testSource, false, assumptions));
+ unit.accept(ConstraintGatherer(typeProvider, _variables, constraints, graph,
+ testSource, false, assumptions));
return unit;
}
}
