Skip to content

Commit

Permalink
Flow analysis: switch on inference based on local boolean variables.
Browse files Browse the repository at this point in the history
The implementation was completed in previous CLs, but hidden behind a
flag.  This CL switches on the feature and adds integration tests.

Bug: dart-lang/language#1274
Change-Id: I0e2d7ac96d3a6d68d33d49f1eab5c341c9167f6f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175500
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
  • Loading branch information
stereotype441 authored and [email protected] committed Jan 8, 2021
1 parent 5b96488 commit 88cd473
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 195 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import 'package:meta/meta.dart';
///
/// Changing this value to `true` will cause some dead code warnings to appear
/// for code that only exists to support the old behavior.
const bool allowLocalBooleanVarsToPromoteByDefault = false;
const bool allowLocalBooleanVarsToPromoteByDefault = true;

/// [AssignedVariables] is a helper class capable of computing the set of
/// variables that are potentially written to, and potentially captured by
Expand Down
188 changes: 0 additions & 188 deletions pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3868,194 +3868,6 @@ main() {
});
});

group('restrict', () {
test('reachability', () {
var h = Harness();
var reachable = FlowModel<Var, Type>(Reachability.initial);
var unreachable = reachable.setUnreachable();
expect(reachable.restrict(h, reachable, Set()), same(reachable));
expect(reachable.restrict(h, unreachable, Set()), same(unreachable));
expect(unreachable.restrict(h, reachable, Set()), same(unreachable));
expect(unreachable.restrict(h, unreachable, Set()), same(unreachable));
});

test('assignments', () {
var h = Harness();
var a = Var('a', 'int');
var b = Var('b', 'int');
var c = Var('c', 'int');
var d = Var('d', 'int');
var s0 = FlowModel<Var, Type>(Reachability.initial)
.declare(a, false)
.declare(b, false)
.declare(c, false)
.declare(d, false);
var s1 = s0
.write(a, Type('int'), new SsaNode<Var, Type>(null), h)
.write(b, Type('int'), new SsaNode<Var, Type>(null), h);
var s2 = s1
.write(a, Type('int'), new SsaNode<Var, Type>(null), h)
.write(c, Type('int'), new SsaNode<Var, Type>(null), h);
var result = s2.restrict(h, s1, Set());
expect(result.infoFor(a).assigned, true);
expect(result.infoFor(b).assigned, true);
expect(result.infoFor(c).assigned, true);
expect(result.infoFor(d).assigned, false);
});

test('write captured', () {
var h = Harness();
var a = Var('a', 'int');
var b = Var('b', 'int');
var c = Var('c', 'int');
var d = Var('d', 'int');
var s0 = FlowModel<Var, Type>(Reachability.initial)
.declare(a, false)
.declare(b, false)
.declare(c, false)
.declare(d, false);
// In s1, a and b are write captured. In s2, a and c are.
var s1 = s0.conservativeJoin([a, b], [a, b]);
var s2 = s1.conservativeJoin([a, c], [a, c]);
var result = s2.restrict(h, s1, Set());
expect(
result.infoFor(a),
_matchVariableModel(writeCaptured: true, unassigned: false),
);
expect(
result.infoFor(b),
_matchVariableModel(writeCaptured: true, unassigned: false),
);
expect(
result.infoFor(c),
_matchVariableModel(writeCaptured: true, unassigned: false),
);
expect(
result.infoFor(d),
_matchVariableModel(writeCaptured: false, unassigned: true),
);
});

test('promotion', () {
void _check(String? thisType, String? otherType, bool unsafe,
List<String>? expectedChain) {
var h = Harness();
var x = Var('x', 'Object?');
var s0 = FlowModel<Var, Type>(Reachability.initial).declare(x, true);
var s1 = thisType == null
? s0
: s0.tryPromoteForTypeCheck(h, x, Type(thisType)).ifTrue;
var s2 = otherType == null
? s0
: s0.tryPromoteForTypeCheck(h, x, Type(otherType)).ifTrue;
var result = s1.restrict(h, s2, unsafe ? [x].toSet() : Set());
if (expectedChain == null) {
expect(result.variableInfo, contains(x));
expect(result.infoFor(x).promotedTypes, isNull);
} else {
expect(result.infoFor(x).promotedTypes!.map((t) => t.type).toList(),
expectedChain);
}
}

_check(null, null, false, null);
_check(null, null, true, null);
_check('int', null, false, ['int']);
_check('int', null, true, ['int']);
_check(null, 'int', false, ['int']);
_check(null, 'int', true, null);
_check('int?', 'int', false, ['int']);
_check('int', 'int?', false, ['int?', 'int']);
_check('int', 'String', false, ['String']);
_check('int?', 'int', true, ['int?']);
_check('int', 'int?', true, ['int']);
_check('int', 'String', true, ['int']);
});

test('promotion chains', () {
// Verify that the given promotion chain matches the expected list of
// strings.
void _checkChain(List<Type>? chain, List<String> expected) {
var strings = (chain ?? <Type>[]).map((t) => t.type).toList();
expect(strings, expected);
}

// Test the following scenario:
// - Prior to the try/finally block, the sequence of promotions in
// [before] is done.
// - During the try block, the sequence of promotions in [inTry] is
// done.
// - During the finally block, the sequence of promotions in
// [inFinally] is done.
// - After calling `restrict` to refine the state from the finally
// block, the expected promotion chain is [expectedResult].
void _check(List<String> before, List<String> inTry,
List<String> inFinally, List<String> expectedResult) {
var h = Harness();
var x = Var('x', 'Object?');
var initialModel =
FlowModel<Var, Type>(Reachability.initial).declare(x, true);
for (var t in before) {
initialModel =
initialModel.tryPromoteForTypeCheck(h, x, Type(t)).ifTrue;
}
_checkChain(initialModel.infoFor(x).promotedTypes, before);
var tryModel = initialModel;
for (var t in inTry) {
tryModel = tryModel.tryPromoteForTypeCheck(h, x, Type(t)).ifTrue;
}
var expectedTryChain = before.toList()..addAll(inTry);
_checkChain(tryModel.infoFor(x).promotedTypes, expectedTryChain);
var finallyModel = initialModel;
for (var t in inFinally) {
finallyModel =
finallyModel.tryPromoteForTypeCheck(h, x, Type(t)).ifTrue;
}
var expectedFinallyChain = before.toList()..addAll(inFinally);
_checkChain(
finallyModel.infoFor(x).promotedTypes, expectedFinallyChain);
var result = finallyModel.restrict(h, tryModel, {});
_checkChain(result.infoFor(x).promotedTypes, expectedResult);
// And verify that the inputs are unchanged.
_checkChain(initialModel.infoFor(x).promotedTypes, before);
_checkChain(tryModel.infoFor(x).promotedTypes, expectedTryChain);
_checkChain(
finallyModel.infoFor(x).promotedTypes, expectedFinallyChain);
}

_check(['Object'], ['Iterable', 'List'], ['num', 'int'],
['Object', 'Iterable', 'List']);
_check([], ['Iterable', 'List'], ['num', 'int'], ['Iterable', 'List']);
_check(['Object'], ['Iterable', 'List'], [],
['Object', 'Iterable', 'List']);
_check([], ['Iterable', 'List'], [], ['Iterable', 'List']);
_check(['Object'], [], ['num', 'int'], ['Object', 'num', 'int']);
_check([], [], ['num', 'int'], ['num', 'int']);
_check(['Object'], [], [], ['Object']);
_check([], [], [], []);
_check(
[], ['Object', 'Iterable'], ['num', 'int'], ['Object', 'Iterable']);
_check([], ['Object'], ['num', 'int'], ['Object', 'num', 'int']);
_check([], ['num', 'int'], ['Object', 'Iterable'], ['num', 'int']);
_check([], ['num', 'int'], ['Object'], ['num', 'int']);
_check([], ['Object', 'int'], ['num'], ['Object', 'int']);
_check([], ['Object', 'num'], ['int'], ['Object', 'num', 'int']);
_check([], ['num'], ['Object', 'int'], ['num', 'int']);
_check([], ['int'], ['Object', 'num'], ['int']);
});

test('variable present in one state but not the other', () {
var h = Harness();
var x = Var('x', 'Object?');
var s0 = FlowModel<Var, Type>(Reachability.initial);
var s1 = s0.declare(x, true);
expect(s0.restrict(h, s1, {}), same(s0));
expect(s0.restrict(h, s1, {x}), same(s0));
expect(s1.restrict(h, s0, {}), same(s0));
expect(s1.restrict(h, s0, {x}), same(s0));
});
});

group('rebaseForward', () {
test('reachability', () {
var h = Harness();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
// Copyright (c) 2020, 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.

finalLocalBool(int? x) {
final bool b = x == null;
if (!b) {
/*nonNullable*/ x;
} else {
x;
}
}

localBool(int? x) {
bool b = x == null;
if (!b) {
/*nonNullable*/ x;
} else {
x;
}
}

localBool_assigned(int? x, bool b1) {
bool b2 = b1;
b2 = x == null;
if (!b2) {
/*nonNullable*/ x;
} else {
x;
}
}

localBool_assignedDynamic(int? x, bool b1) {
dynamic b2 = b1;
b2 = x == null;
if (!b2) {
/*nonNullable*/ x;
} else {
x;
}
}

parameter_assigned(int? x, bool b) {
b = x == null;
if (!b) {
/*nonNullable*/ x;
} else {
x;
}
}

parameter_assignedDynamic(int? x, dynamic b) {
b = x == null;
if (!b) {
/*nonNullable*/ x;
} else {
x;
}
}

lateFinalLocalBool(int? x) {
late final bool b = x == null;
if (!b) {
// We don't promote based on the initializers of late locals because we
// don't know when they execute.
x;
} else {
x;
}
}

lateLocalBool(int? x) {
late bool b = x == null;
if (!b) {
// We don't promote based on the initializers of late locals because we
// don't know when they execute.
x;
} else {
x;
}
}

lateLocalBool_assignedAndInitialized(int? x, bool b1) {
late bool b2 = b1;
b2 = x == null;
if (!b2) {
/*nonNullable*/ x;
} else {
x;
}
}

lateLocalBool_assignedButNotInitialized(int? x) {
late bool b;
b = x == null;
if (!b) {
/*nonNullable*/ x;
} else {
x;
}
}

rebaseWithDemotion(int? x, int? y, int? z, int? a) {
x;
y;
z;
if (y == null) return;
x;
/*nonNullable*/ y;
z;
bool b = x == null;
x;
/*nonNullable*/ y;
z;
if (z == null) return;
x;
/*nonNullable*/ y;
/*nonNullable*/ z;
y = a;
x;
y;
/*nonNullable*/ z;
if (b) return;
/*nonNullable*/ x;
y;
/*nonNullable*/ z;
}

compoundAssignment(int? x, dynamic b) {
b += x == null;
if (!b) {
// It's not safe to promote, because there's no guarantee that value of `b`
// has anything to do with the result of `x == null`.
x;
} else {
x;
}
}

ifNullAssignment(int? x, dynamic b) {
b ??= x == null;
if (!b) {
// It's not safe to promote, because there's no guarantee that value of `b`
// has anything to do with the result of `x == null`.
x;
} else {
x;
}
}
18 changes: 16 additions & 2 deletions pkg/analyzer/test/id_tests/nullability_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:analyzer/dart/analysis/features.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/null_safety_understanding_flag.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/dart/element/type_system.dart';
import 'package:analyzer/src/dart/analysis/testing_data.dart';
import 'package:analyzer/src/dart/element/type.dart';
Expand Down Expand Up @@ -78,9 +79,9 @@ class _NullabilityDataExtractor extends AstDataExtractor<String> {
!node.inDeclarationContext()) {
var element = node.staticElement;
if (element is LocalVariableElement || element is ParameterElement) {
TypeImpl promotedType = node.staticType;
TypeImpl promotedType = _readType(node);
TypeImpl declaredType = (element as VariableElement).type;
var isPromoted = promotedType != declaredType;
var isPromoted = promotedType != null && promotedType != declaredType;
if (isPromoted &&
_typeSystem.isPotentiallyNullable(declaredType) &&
!_typeSystem.isPotentiallyNullable(promotedType)) {
Expand All @@ -90,6 +91,19 @@ class _NullabilityDataExtractor extends AstDataExtractor<String> {
}
return null;
}

static DartType _readType(SimpleIdentifier node) {
var parent = node.parent;
if (parent is AssignmentExpression && parent.leftHandSide == node) {
return parent.readType;
} else if (parent is PostfixExpression) {
return parent.readType;
} else if (parent is PrefixExpression) {
return parent.readType;
} else {
return node.staticType;
}
}
}

class _NullabilityDataInterpreter implements DataInterpreter<String> {
Expand Down
Loading

0 comments on commit 88cd473

Please sign in to comment.