From c5567c0031d8738c280d89c7f12d7f53b5fcd40d Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Wed, 19 Jun 2019 22:51:11 +0000 Subject: [PATCH] Abstract FlowAnalysis from concrete types of nodes and elements. R=paulberry@google.com Change-Id: I8a42eb260d2c99c4ca24cb8b9059c1d22f9430e9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/106683 Commit-Queue: Konstantin Shcheglov Reviewed-by: Paul Berry --- .../lib/src/dart/resolver/flow_analysis.dart | 450 ++++++++++-------- .../dart/resolution/flow_analysis_test.dart | 108 +++-- .../lib/src/decorated_type_operations.dart | 8 +- 3 files changed, 343 insertions(+), 223 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/resolver/flow_analysis.dart b/pkg/analyzer/lib/src/dart/resolver/flow_analysis.dart index 1c8dc319693..b711bcbb6bb 100644 --- a/pkg/analyzer/lib/src/dart/resolver/flow_analysis.dart +++ b/pkg/analyzer/lib/src/dart/resolver/flow_analysis.dart @@ -2,62 +2,60 @@ // 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:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/element/element.dart'; +/// Sets of variables that are potentially assigned in a statement. +class AssignedVariables { + final emptySet = Set(); -/// Sets of variables that are potentially assigned in a node. -class AssignedVariables { - static final emptySet = Set(); + /// Mapping from a [Statement] representing a loop to the set of variables + /// that are potentially assigned in that loop. + final Map> _map = {}; - /// Mapping from a loop [AstNode] to the set of variables that are - /// potentially assigned in this loop. - final Map> _map = {}; + /// The stack of nested nodes. + final List> _stack = []; - /// The stack of nested loops. - final List> _stack = []; + AssignedVariables(); - /// Return the set of variables that are potentially assigned in the [loop]. - Set operator [](AstNode loop) { - return _map[loop] ?? emptySet; + /// Return the set of variables that are potentially assigned in the + /// [statement]. + Set operator [](Statement statement) { + return _map[statement] ?? emptySet; } void beginLoop() { - var set = Set.identity(); + var set = Set.identity(); _stack.add(set); } - void endLoop(AstNode loop) { - _map[loop] = _stack.removeLast(); + void endLoop(Statement node) { + _map[node] = _stack.removeLast(); } - void write(VariableElement variable) { + void write(Element variable) { for (var i = 0; i < _stack.length; ++i) { _stack[i].add(variable); } } } -class FlowAnalysis { - final _identity = _State( - false, - _ElementSet.empty, - _ElementSet.empty, - _ElementSet.empty, - const {}, - ); +class FlowAnalysis { + final _ElementSet _emptySet; + final _State _identity; /// The output list of variables that were read before they were written. /// TODO(scheglov) use _ElementSet? - final List readBeforeWritten = []; + final List readBeforeWritten = []; + + /// The [NodeOperations], used to manipulate expressions. + final NodeOperations nodeOperations; /// The [TypeOperations], used to access types, and check subtyping. - final TypeOperations typeOperations; + final TypeOperations typeOperations; - /// The enclosing [FunctionBody], used to check for potential mutations. - final FunctionBody functionBody; + /// The enclosing function body, used to check for potential mutations. + final FunctionBodyAccess functionBody; /// The stack of states of variables that are not definitely assigned. - final List<_State> _stack = []; + final List<_State> _stack = []; /// The mapping from labeled [Statement]s to the index in the [_stack] /// where the first related element is located. The number of elements @@ -66,25 +64,55 @@ class FlowAnalysis { final Map _statementToStackIndex = {}; /// The list of all variables. - final List _variables = []; + final List _variables = []; - _State _current; + _State _current; /// The last boolean condition, for [_conditionTrue] and [_conditionFalse]. Expression _condition; /// The state when [_condition] evaluates to `true`. - _State _conditionTrue; + _State _conditionTrue; /// The state when [_condition] evaluates to `false`. - _State _conditionFalse; + _State _conditionFalse; - FlowAnalysis(this.typeOperations, this.functionBody) { - _current = _State( + factory FlowAnalysis( + NodeOperations nodeOperations, + TypeOperations typeOperations, + FunctionBodyAccess functionBody, + ) { + var emptySet = _ElementSet._( + List(0), + ); + var identifyState = _State( + false, + emptySet, + emptySet, + emptySet, + const {}, + ); + return FlowAnalysis._( + nodeOperations, + typeOperations, + functionBody, + emptySet, + identifyState, + ); + } + + FlowAnalysis._( + this.nodeOperations, + this.typeOperations, + this.functionBody, + this._emptySet, + this._identity, + ) { + _current = _State( true, - _ElementSet.empty, - _ElementSet.empty, - _ElementSet.empty, + _emptySet, + _emptySet, + _emptySet, const {}, ); } @@ -93,17 +121,18 @@ class FlowAnalysis { bool get isReachable => _current.reachable; /// Add a new [variable], which might be already [assigned]. - void add(VariableElement variable, {bool assigned: false}) { + void add(Element variable, {bool assigned: false}) { _variables.add(variable); _current = _current.add(variable, assigned: assigned); } - void conditional_elseBegin(ConditionalExpression node, bool isBool) { + void conditional_elseBegin(Expression conditionalExpression, + Expression thenExpression, bool isBool) { var afterThen = _current; var falseCondition = _stack.removeLast(); if (isBool) { - _conditionalEnd(node.thenExpression); + _conditionalEnd(thenExpression); // Tail of the stack: falseThen, trueThen } @@ -111,12 +140,13 @@ class FlowAnalysis { _current = falseCondition; } - void conditional_end(ConditionalExpression node, bool isBool) { + void conditional_end(Expression conditionalExpression, + Expression elseExpression, bool isBool) { var afterThen = _stack.removeLast(); var afterElse = _current; if (isBool) { - _conditionalEnd(node.elseExpression); + _conditionalEnd(elseExpression); // Tail of the stack: falseThen, trueThen, falseElse, trueElse var trueElse = _stack.removeLast(); @@ -128,7 +158,7 @@ class FlowAnalysis { var trueResult = _join(trueThen, trueElse); var falseResult = _join(falseThen, falseElse); - _condition = node; + _condition = conditionalExpression; _conditionTrue = trueResult; _conditionFalse = falseResult; } @@ -136,41 +166,41 @@ class FlowAnalysis { _current = _join(afterThen, afterElse); } - void conditional_thenBegin(ConditionalExpression node) { - _conditionalEnd(node.condition); + void conditional_thenBegin( + Expression conditionalExpression, Expression condition) { + _conditionalEnd(condition); // Tail of the stack: falseCondition, trueCondition var trueCondition = _stack.removeLast(); _current = trueCondition; } - /// The [node] checks that the [variable] is equal to `null`. - void conditionEqNull(BinaryExpression node, VariableElement variable) { + /// The [binaryExpression] checks that the [variable] is equal to `null`. + void conditionEqNull(Expression binaryExpression, Element variable) { if (functionBody.isPotentiallyMutatedInClosure(variable)) { return; } - _condition = node; - _conditionTrue = _current.markNullable(variable); - _conditionFalse = _current.markNonNullable(variable); + _condition = binaryExpression; + _conditionTrue = _current.markNullable(_emptySet, variable); + _conditionFalse = _current.markNonNullable(_emptySet, variable); } - /// The [node] checks that the [variable] is not equal to `null`. - void conditionNotEqNull(BinaryExpression node, VariableElement variable) { + /// The [binaryExpression] checks that the [variable] is not equal to `null`. + void conditionNotEqNull(Expression binaryExpression, Element variable) { if (functionBody.isPotentiallyMutatedInClosure(variable)) { return; } - _condition = node; - _conditionTrue = _current.markNonNullable(variable); - _conditionFalse = _current.markNullable(variable); + _condition = binaryExpression; + _conditionTrue = _current.markNonNullable(_emptySet, variable); + _conditionFalse = _current.markNullable(_emptySet, variable); } - void doStatement_bodyBegin( - DoStatement node, Set loopAssigned) { + void doStatement_bodyBegin(Statement doStatement, Set loopAssigned) { _current = _current.removePromotedAll(loopAssigned); - _statementToStackIndex[node] = _stack.length; + _statementToStackIndex[doStatement] = _stack.length; _stack.add(_identity); // break _stack.add(_identity); // continue } @@ -182,8 +212,8 @@ class FlowAnalysis { _current = _join(_current, continueState); } - void doStatement_end(DoStatement node) { - _conditionalEnd(node.condition); + void doStatement_end(Statement doStatement, Expression condition) { + _conditionalEnd(condition); // Tail of the stack: break, falseCondition, trueCondition _stack.removeLast(); // trueCondition @@ -193,13 +223,13 @@ class FlowAnalysis { _current = _join(falseCondition, breakState); } - void falseLiteral(BooleanLiteral expression) { + void falseLiteral(Expression expression) { _condition = expression; _conditionTrue = _identity; _conditionFalse = _current; } - void forEachStatement_bodyBegin(Set loopAssigned) { + void forEachStatement_bodyBegin(Set loopAssigned) { _stack.add(_current); _current = _current.removePromotedAll(loopAssigned); } @@ -222,7 +252,7 @@ class FlowAnalysis { _current = trueCondition; } - void forStatement_conditionBegin(Set loopAssigned) { + void forStatement_conditionBegin(Set loopAssigned) { _current = _current.removePromotedAll(loopAssigned); } @@ -245,10 +275,10 @@ class FlowAnalysis { void functionExpression_begin() { _stack.add(_current); - Set notPromoted = null; + Set notPromoted = null; for (var variable in _current.promoted.keys) { if (functionBody.isPotentiallyMutatedInScope(variable)) { - notPromoted ??= Set.identity(); + notPromoted ??= Set.identity(); notPromoted.add(variable); } } @@ -262,7 +292,7 @@ class FlowAnalysis { _current = _stack.removeLast(); } - void handleBreak(AstNode target) { + void handleBreak(Statement target) { var breakIndex = _statementToStackIndex[target]; if (breakIndex != null) { _stack[breakIndex] = _join(_stack[breakIndex], _current); @@ -270,7 +300,7 @@ class FlowAnalysis { _current = _current.exit(); } - void handleContinue(AstNode target) { + void handleContinue(Statement target) { var breakIndex = _statementToStackIndex[target]; if (breakIndex != null) { var continueIndex = breakIndex + 1; @@ -302,8 +332,8 @@ class FlowAnalysis { } void ifStatement_end(bool hasElse) { - _State afterThen; - _State afterElse; + _State afterThen; + _State afterElse; if (hasElse) { afterThen = _stack.removeLast(); afterElse = _current; @@ -314,8 +344,8 @@ class FlowAnalysis { _current = _join(afterThen, afterElse); } - void ifStatement_thenBegin(IfStatement ifStatement) { - _conditionalEnd(ifStatement.condition); + void ifStatement_thenBegin(Statement ifStatement, Expression condition) { + _conditionalEnd(condition); // Tail of the stack: falseCondition, trueCondition var trueCondition = _stack.removeLast(); @@ -323,33 +353,33 @@ class FlowAnalysis { } void isExpression_end( - IsExpression isExpression, VariableElement variable, T type) { + Expression isExpression, Element variable, bool isNot, Type type) { if (functionBody.isPotentiallyMutatedInClosure(variable)) { return; } _condition = isExpression; - if (isExpression.notOperator == null) { - _conditionTrue = _current.promote(typeOperations, variable, type); - _conditionFalse = _current; - } else { + if (isNot) { _conditionTrue = _current; _conditionFalse = _current.promote(typeOperations, variable, type); + } else { + _conditionTrue = _current.promote(typeOperations, variable, type); + _conditionFalse = _current; } } /// Return `true` if the [variable] is known to be be nullable. - bool isNonNullable(VariableElement variable) { + bool isNonNullable(Element variable) { return !_current.notNonNullable.contains(variable); } /// Return `true` if the [variable] is known to be be non-nullable. - bool isNullable(VariableElement variable) { + bool isNullable(Element variable) { return !_current.notNullable.contains(variable); } - void logicalAnd_end(BinaryExpression andExpression) { - _conditionalEnd(andExpression.rightOperand); + void logicalAnd_end(Expression andExpression, Expression rightOperand) { + _conditionalEnd(rightOperand); // Tail of the stack: falseLeft, trueLeft, falseRight, trueRight var trueRight = _stack.removeLast(); @@ -369,16 +399,16 @@ class FlowAnalysis { _current = afterResult; } - void logicalAnd_rightBegin(BinaryExpression andExpression) { - _conditionalEnd(andExpression.leftOperand); + void logicalAnd_rightBegin(Expression andExpression, Expression leftOperand) { + _conditionalEnd(leftOperand); // Tail of the stack: falseLeft, trueLeft var trueLeft = _stack.last; _current = trueLeft; } - void logicalNot_end(PrefixExpression notExpression) { - _conditionalEnd(notExpression.operand); + void logicalNot_end(Expression notExpression, Expression operand) { + _conditionalEnd(operand); var trueExpr = _stack.removeLast(); var falseExpr = _stack.removeLast(); @@ -387,8 +417,8 @@ class FlowAnalysis { _conditionFalse = trueExpr; } - void logicalOr_end(BinaryExpression orExpression) { - _conditionalEnd(orExpression.rightOperand); + void logicalOr_end(Expression orExpression, Expression rightOperand) { + _conditionalEnd(rightOperand); // Tail of the stack: falseLeft, trueLeft, falseRight, trueRight var trueRight = _stack.removeLast(); @@ -408,8 +438,8 @@ class FlowAnalysis { _current = afterResult; } - void logicalOr_rightBegin(BinaryExpression orExpression) { - _conditionalEnd(orExpression.leftOperand); + void logicalOr_rightBegin(Expression orExpression, Expression leftOperand) { + _conditionalEnd(leftOperand); // Tail of the stack: falseLeft, trueLeft var falseLeft = _stack[_stack.length - 2]; @@ -418,12 +448,12 @@ class FlowAnalysis { /// Retrieves the type that the [variable] is promoted to, if the [variable] /// is currently promoted. Otherwise returns `null`. - T promotedType(VariableElement variable) { + Type promotedType(Element variable) { return _current.promoted[variable]; } /// Register read of the given [variable] in the current state. - void read(LocalVariableElement variable) { + void read(Element variable) { if (_current.notAssigned.contains(variable)) { // Add to the list of violating variables, if not there yet. for (var i = 0; i < readBeforeWritten.length; ++i) { @@ -440,11 +470,11 @@ class FlowAnalysis { /// assigned in other cases that might target this with `continue`, so /// these variables might have different types and are "un-promoted" from /// the "afterExpression" state. - void switchStatement_beginCase(Set notPromoted) { + void switchStatement_beginCase(Set notPromoted) { _current = _stack.last.removePromotedAll(notPromoted); } - void switchStatement_end(SwitchStatement node, bool hasDefault) { + void switchStatement_end(Statement switchStatement, bool hasDefault) { // Tail of the stack: break, continue, afterExpression var afterExpression = _current = _stack.removeLast(); _stack.removeLast(); // continue @@ -457,14 +487,14 @@ class FlowAnalysis { } } - void switchStatement_expressionEnd(SwitchStatement node) { - _statementToStackIndex[node] = _stack.length; + void switchStatement_expressionEnd(Statement switchStatement) { + _statementToStackIndex[switchStatement] = _stack.length; _stack.add(_identity); // break _stack.add(_identity); // continue _stack.add(_current); // afterExpression } - void trueLiteral(BooleanLiteral expression) { + void trueLiteral(Expression expression) { _condition = expression; _conditionTrue = _current; _conditionFalse = _identity; @@ -475,7 +505,7 @@ class FlowAnalysis { // Tail of the stack: beforeBody } - void tryCatchStatement_bodyEnd(Set assignedInBody) { + void tryCatchStatement_bodyEnd(Set assignedInBody) { var beforeBody = _stack.removeLast(); var beforeCatch = beforeBody.removePromotedAll(assignedInBody); _stack.add(beforeCatch); @@ -503,12 +533,17 @@ class FlowAnalysis { _stack.add(_current); // beforeTry } - void tryFinallyStatement_end(Set assignedInFinally) { + void tryFinallyStatement_end(Set assignedInFinally) { var afterBody = _stack.removeLast(); - _current = _current.restrict(typeOperations, afterBody, assignedInFinally); + _current = _current.restrict( + typeOperations, + _emptySet, + afterBody, + assignedInFinally, + ); } - void tryFinallyStatement_finallyBegin(Set assignedInBody) { + void tryFinallyStatement_finallyBegin(Set assignedInBody) { var beforeTry = _stack.removeLast(); var afterBody = _current; _stack.add(afterBody); @@ -519,20 +554,21 @@ class FlowAnalysis { assert(_stack.isEmpty); } - void whileStatement_bodyBegin(WhileStatement node) { - _conditionalEnd(node.condition); + void whileStatement_bodyBegin( + Statement whileStatement, Expression condition) { + _conditionalEnd(condition); // Tail of the stack: falseCondition, trueCondition var trueCondition = _stack.removeLast(); - _statementToStackIndex[node] = _stack.length; + _statementToStackIndex[whileStatement] = _stack.length; _stack.add(_identity); // break _stack.add(_identity); // continue _current = trueCondition; } - void whileStatement_conditionBegin(Set loopAssigned) { + void whileStatement_conditionBegin(Set loopAssigned) { _current = _current.removePromotedAll(loopAssigned); } @@ -546,17 +582,16 @@ class FlowAnalysis { /// Register write of the given [variable] in the current state. void write( - VariableElement variable, { + Element variable, { bool isNull = false, bool isNonNull = false, }) { - _current = _current.write(variable, isNull: isNull, isNonNull: isNonNull); + _current = _current.write(typeOperations, _emptySet, variable, + isNull: isNull, isNonNull: isNonNull); } void _conditionalEnd(Expression condition) { - while (condition is ParenthesizedExpression) { - condition = (condition as ParenthesizedExpression).expression; - } + condition = nodeOperations.unwrapParenthesized(condition); if (identical(condition, _condition)) { _stack.add(_conditionFalse); _stack.add(_conditionTrue); @@ -566,7 +601,10 @@ class FlowAnalysis { } } - _State _join(_State first, _State second) { + _State _join( + _State first, + _State second, + ) { if (identical(first, _identity)) return second; if (identical(second, _identity)) return first; @@ -590,14 +628,14 @@ class FlowAnalysis { ); } - Map _joinPromoted( - Map first, - Map second, + Map _joinPromoted( + Map first, + Map second, ) { if (identical(first, second)) return first; if (first.isEmpty || second.isEmpty) return const {}; - var result = {}; + var result = {}; var alwaysFirst = true; var alwaysSecond = true; for (var element in first.keys) { @@ -627,32 +665,44 @@ class FlowAnalysis { } } +/// Accessor for function body information. +abstract class FunctionBodyAccess { + bool isPotentiallyMutatedInClosure(Element variable); + + bool isPotentiallyMutatedInScope(Element variable); +} + +/// Operations on nodes, abstracted from concrete node interfaces. +abstract class NodeOperations { + /// If the [node] is a parenthesized expression, recursively unwrap it. + Expression unwrapParenthesized(Expression node); +} + /// Operations on types, abstracted from concrete type interfaces. -abstract class TypeOperations { - /// Return the static type of with the given [element]. - T elementType(VariableElement element); +abstract class TypeOperations { + /// Return the static type of the given [element]. + Type elementType(Element element); + + /// Return `true` if the [element] is a local variable, not a parameter. + bool isLocalVariable(Element element); /// Return `true` if the [leftType] is a subtype of the [rightType]. - bool isSubtypeOf(T leftType, T rightType); + bool isSubtypeOf(Type leftType, Type rightType); } /// List based immutable set of elements. -class _ElementSet { - static final empty = _ElementSet._( - List(0), - ); - - final List elements; +class _ElementSet { + final List elements; _ElementSet._(this.elements); - _ElementSet add(VariableElement addedElement) { + _ElementSet add(Element addedElement) { if (contains(addedElement)) { return this; } var length = elements.length; - var newElements = List(length + 1); + var newElements = List(length + 1); for (var i = 0; i < length; ++i) { newElements[i] = elements[i]; } @@ -660,7 +710,7 @@ class _ElementSet { return _ElementSet._(newElements); } - _ElementSet addAll(Iterable elements) { + _ElementSet addAll(Iterable elements) { var result = this; for (var element in elements) { result = result.add(element); @@ -668,7 +718,7 @@ class _ElementSet { return result; } - bool contains(VariableElement element) { + bool contains(Element element) { var length = elements.length; for (var i = 0; i < length; ++i) { if (identical(elements[i], element)) { @@ -678,7 +728,10 @@ class _ElementSet { return false; } - _ElementSet intersect(_ElementSet other) { + _ElementSet intersect({ + _ElementSet empty, + _ElementSet other, + }) { if (identical(other, empty)) return empty; // TODO(scheglov) optimize @@ -689,7 +742,10 @@ class _ElementSet { return _ElementSet._(newElements); } - _ElementSet remove(VariableElement removedElement) { + _ElementSet remove( + _ElementSet empty, + Element removedElement, + ) { if (!contains(removedElement)) { return this; } @@ -699,7 +755,7 @@ class _ElementSet { return empty; } - var newElements = List(length - 1); + var newElements = List(length - 1); var newIndex = 0; for (var i = 0; i < length; ++i) { var element = elements[i]; @@ -711,7 +767,7 @@ class _ElementSet { return _ElementSet._(newElements); } - _ElementSet union(_ElementSet other) { + _ElementSet union(_ElementSet other) { if (other.elements.isEmpty) { return this; } @@ -726,12 +782,12 @@ class _ElementSet { } } -class _State { +class _State { final bool reachable; - final _ElementSet notAssigned; - final _ElementSet notNullable; - final _ElementSet notNonNullable; - final Map promoted; + final _ElementSet notAssigned; + final _ElementSet notNullable; + final _ElementSet notNonNullable; + final Map promoted; _State( this.reachable, @@ -742,7 +798,7 @@ class _State { ); /// Add a new [variable] to track definite assignment. - _State add(VariableElement variable, {bool assigned: false}) { + _State add(Element variable, {bool assigned: false}) { var newNotAssigned = assigned ? notAssigned : notAssigned.add(variable); var newNotNullable = notNullable.add(variable); var newNotNonNullable = notNonNullable.add(variable); @@ -753,7 +809,7 @@ class _State { return this; } - return _State( + return _State( reachable, newNotAssigned, newNotNullable, @@ -762,20 +818,27 @@ class _State { ); } - _State exit() { - return _State(false, notAssigned, notNullable, notNonNullable, promoted); + _State exit() { + return _State( + false, + notAssigned, + notNullable, + notNonNullable, + promoted, + ); } - _State markNonNullable(VariableElement variable) { + _State markNonNullable( + _ElementSet emptySet, Element variable) { var newNotNullable = notNullable.add(variable); - var newNotNonNullable = notNonNullable.remove(variable); + var newNotNonNullable = notNonNullable.remove(emptySet, variable); if (identical(newNotNullable, notNullable) && identical(newNotNonNullable, notNonNullable)) { return this; } - return _State( + return _State( reachable, notAssigned, newNotNullable, @@ -784,8 +847,9 @@ class _State { ); } - _State markNullable(VariableElement variable) { - var newNotNullable = notNullable.remove(variable); + _State markNullable( + _ElementSet emptySet, Element variable) { + var newNotNullable = notNullable.remove(emptySet, variable); var newNotNonNullable = notNonNullable.add(variable); if (identical(newNotNullable, notNullable) && @@ -793,7 +857,7 @@ class _State { return this; } - return _State( + return _State( reachable, notAssigned, newNotNullable, @@ -802,19 +866,19 @@ class _State { ); } - _State promote( - TypeOperations typeOperations, - VariableElement variable, - T type, + _State promote( + TypeOperations typeOperations, + Element variable, + Type type, ) { var previousType = promoted[variable]; previousType ??= typeOperations.elementType(variable); if (typeOperations.isSubtypeOf(type, previousType) && type != previousType) { - var newPromoted = {}..addAll(promoted); + var newPromoted = {}..addAll(promoted); newPromoted[variable] = type; - return _State( + return _State( reachable, notAssigned, notNullable, @@ -826,7 +890,7 @@ class _State { return this; } - _State removePromotedAll(Set variables) { + _State removePromotedAll(Set variables) { var newNotNullable = notNullable.addAll(variables); var newNotNonNullable = notNonNullable.addAll(variables); var newPromoted = _removePromotedAll(promoted, variables); @@ -835,7 +899,7 @@ class _State { identical(newNotNonNullable, notNonNullable) && identical(newPromoted, promoted)) return this; - return _State( + return _State( reachable, notAssigned, newNotNullable, @@ -844,22 +908,26 @@ class _State { ); } - _State restrict( - TypeOperations typeOperations, - _State other, - Set unsafe, + _State restrict( + TypeOperations typeOperations, + _ElementSet emptySet, + _State other, + Set unsafe, ) { var newReachable = reachable && other.reachable; - var newNotAssigned = notAssigned.intersect(other.notAssigned); + var newNotAssigned = notAssigned.intersect( + empty: emptySet, + other: other.notAssigned, + ); - var newNotNullable = _ElementSet.empty; + var newNotNullable = emptySet; for (var variable in notNullable.elements) { if (unsafe.contains(variable) || other.notNullable.contains(variable)) { newNotNullable = newNotNullable.add(variable); } } - var newNotNonNullable = _ElementSet.empty; + var newNotNonNullable = emptySet; for (var variable in notNonNullable.elements) { if (unsafe.contains(variable) || other.notNonNullable.contains(variable)) { @@ -867,7 +935,7 @@ class _State { } } - var newPromoted = {}; + var newPromoted = {}; for (var variable in promoted.keys) { var thisType = promoted[variable]; if (!unsafe.contains(variable)) { @@ -892,10 +960,10 @@ class _State { ); } - _State setReachable(bool reachable) { + _State setReachable(bool reachable) { if (this.reachable == reachable) return this; - return _State( + return _State( reachable, notAssigned, notNullable, @@ -904,20 +972,23 @@ class _State { ); } - _State write( - VariableElement variable, { + _State write( + TypeOperations typeOperations, + _ElementSet emptySet, + Element variable, { bool isNull = false, bool isNonNull = false, }) { - var newNotAssigned = variable is LocalVariableElement - ? notAssigned.remove(variable) + var newNotAssigned = typeOperations.isLocalVariable(variable) + ? notAssigned.remove(emptySet, variable) : notAssigned; - var newNotNullable = - isNull ? notNullable.remove(variable) : notNullable.add(variable); + var newNotNullable = isNull + ? notNullable.remove(emptySet, variable) + : notNullable.add(variable); var newNotNonNullable = isNonNull - ? notNonNullable.remove(variable) + ? notNonNullable.remove(emptySet, variable) : notNonNullable.add(variable); var newPromoted = _removePromoted(promoted, variable); @@ -929,7 +1000,7 @@ class _State { return this; } - return _State( + return _State( reachable, newNotAssigned, newNotNullable, @@ -938,13 +1009,10 @@ class _State { ); } - Map _removePromoted( - Map map, - VariableElement variable, - ) { + Map _removePromoted(Map map, Element variable) { if (map.isEmpty) return const {}; - var result = {}; + var result = {}; for (var key in map.keys) { if (!identical(key, variable)) { result[key] = map[key]; @@ -955,14 +1023,14 @@ class _State { return result; } - Map _removePromotedAll( - Map map, - Set variables, + Map _removePromotedAll( + Map map, + Set variables, ) { if (map.isEmpty) return const {}; if (variables.isEmpty) return map; - var result = {}; + var result = {}; var noChanges = true; for (var key in map.keys) { if (variables.contains(key)) { @@ -977,14 +1045,14 @@ class _State { return result; } - static _State _identicalOrNew( - _State first, - _State second, + static _State _identicalOrNew( + _State first, + _State second, bool newReachable, - _ElementSet newNotAssigned, - _ElementSet newNotNullable, - _ElementSet newNotNonNullable, - Map newPromoted, + _ElementSet newNotAssigned, + _ElementSet newNotNullable, + _ElementSet newNotNonNullable, + Map newPromoted, ) { if (first.reachable == newReachable && identical(first.notAssigned, newNotAssigned) && @@ -1001,7 +1069,7 @@ class _State { return second; } - return _State( + return _State( newReachable, newNotAssigned, newNotNullable, diff --git a/pkg/analyzer/test/src/dart/resolution/flow_analysis_test.dart b/pkg/analyzer/test/src/dart/resolution/flow_analysis_test.dart index 5dd9420a664..6a39734c92f 100644 --- a/pkg/analyzer/test/src/dart/resolution/flow_analysis_test.dart +++ b/pkg/analyzer/test/src/dart/resolution/flow_analysis_test.dart @@ -1308,13 +1308,13 @@ void f() { var unit = result.unit; - var loopAssignedVariables = AssignedVariables(); - unit.accept(_AssignedVariablesVisitor(loopAssignedVariables)); + var assignedVariables = AssignedVariables(); + unit.accept(_AssignedVariablesVisitor(assignedVariables)); var typeSystem = unit.declaredElement.context.typeSystem; unit.accept(_AstVisitor( typeSystem, - loopAssignedVariables, + assignedVariables, {}, readBeforeWritten, [], @@ -1623,13 +1623,13 @@ void f(int x) { var unit = result.unit; - var loopAssignedVariables = AssignedVariables(); - unit.accept(_AssignedVariablesVisitor(loopAssignedVariables)); + var assignedVariables = AssignedVariables(); + unit.accept(_AssignedVariablesVisitor(assignedVariables)); var typeSystem = unit.declaredElement.context.typeSystem; unit.accept(_AstVisitor( typeSystem, - loopAssignedVariables, + assignedVariables, {}, [], nullableNodes, @@ -2070,13 +2070,13 @@ void f() { // f var unit = result.unit; - var loopAssignedVariables = AssignedVariables(); - unit.accept(_AssignedVariablesVisitor(loopAssignedVariables)); + var assignedVariables = AssignedVariables(); + unit.accept(_AssignedVariablesVisitor(assignedVariables)); var typeSystem = unit.declaredElement.context.typeSystem; unit.accept(_AstVisitor( typeSystem, - loopAssignedVariables, + assignedVariables, {}, [], [], @@ -2903,13 +2903,13 @@ void f(bool b, Object x) { var unit = result.unit; - var loopAssignedVariables = AssignedVariables(); - unit.accept(_AssignedVariablesVisitor(loopAssignedVariables)); + var assignedVariables = AssignedVariables(); + unit.accept(_AssignedVariablesVisitor(assignedVariables)); var typeSystem = unit.declaredElement.context.typeSystem; unit.accept(_AstVisitor( typeSystem, - loopAssignedVariables, + assignedVariables, promotedTypes, [], [], @@ -3018,7 +3018,8 @@ class _AssignedVariablesVisitor extends RecursiveAstVisitor { class _AstVisitor extends GeneralizingAstVisitor { static final trueLiteral = astFactory.booleanLiteral(null, true); - final TypeOperations typeOperations; + final NodeOperations nodeOperations; + final TypeOperations typeOperations; final AssignedVariables assignedVariables; final Map promotedTypes; final List readBeforeWritten; @@ -3027,7 +3028,7 @@ class _AstVisitor extends GeneralizingAstVisitor { final List unreachableNodes; final List functionBodiesThatDontComplete; - FlowAnalysis flow; + FlowAnalysis flow; _AstVisitor( TypeSystem typeSystem, @@ -3038,7 +3039,8 @@ class _AstVisitor extends GeneralizingAstVisitor { this.nonNullableNodes, this.unreachableNodes, this.functionBodiesThatDontComplete) - : typeOperations = _TypeSystemTypeOperations(typeSystem); + : nodeOperations = _NodeOperations(), + typeOperations = _TypeSystemTypeOperations(typeSystem); @override void visitAssignmentExpression(AssignmentExpression node) { @@ -3080,19 +3082,19 @@ class _AstVisitor extends GeneralizingAstVisitor { if (operator == TokenType.AMPERSAND_AMPERSAND) { left.accept(this); - flow.logicalAnd_rightBegin(node); + flow.logicalAnd_rightBegin(node, node.leftOperand); _checkUnreachableNode(node.rightOperand); right.accept(this); - flow.logicalAnd_end(node); + flow.logicalAnd_end(node, node.rightOperand); } else if (operator == TokenType.BAR_BAR) { left.accept(this); - flow.logicalOr_rightBegin(node); + flow.logicalOr_rightBegin(node, node.leftOperand); _checkUnreachableNode(node.rightOperand); right.accept(this); - flow.logicalOr_end(node); + flow.logicalOr_end(node, node.rightOperand); } else if (operator == TokenType.BANG_EQ) { left.accept(this); right.accept(this); @@ -3133,7 +3135,11 @@ class _AstVisitor extends GeneralizingAstVisitor { var isFlowOwner = flow == null; if (isFlowOwner) { - flow = FlowAnalysis(typeOperations, node); + flow = FlowAnalysis( + nodeOperations, + typeOperations, + _FunctionBodyAccess(node), + ); var function = node.parent; if (function is FunctionExpression) { @@ -3149,7 +3155,10 @@ class _AstVisitor extends GeneralizingAstVisitor { super.visitBlockFunctionBody(node); if (isFlowOwner) { - readBeforeWritten.addAll(flow.readBeforeWritten); + for (var variable in flow.readBeforeWritten) { + assert(variable is LocalVariableElement); + readBeforeWritten.add(variable); + } if (!flow.isReachable) { functionBodiesThatDontComplete.add(node); @@ -3186,16 +3195,16 @@ class _AstVisitor extends GeneralizingAstVisitor { condition.accept(this); - flow.conditional_thenBegin(node); + flow.conditional_thenBegin(node, node.condition); _checkUnreachableNode(node.thenExpression); thenExpression.accept(this); var isBool = thenExpression.staticType.isDartCoreBool; - flow.conditional_elseBegin(node, isBool); + flow.conditional_elseBegin(node, node.thenExpression, isBool); _checkUnreachableNode(node.elseExpression); elseExpression.accept(this); - flow.conditional_end(node, isBool); + flow.conditional_end(node, node.elseExpression, isBool); } @override @@ -3218,7 +3227,7 @@ class _AstVisitor extends GeneralizingAstVisitor { flow.doStatement_conditionBegin(); condition.accept(this); - flow.doStatement_end(node); + flow.doStatement_end(node, node.condition); } @override @@ -3285,7 +3294,7 @@ class _AstVisitor extends GeneralizingAstVisitor { condition.accept(this); - flow.ifStatement_thenBegin(node); + flow.ifStatement_thenBegin(node, node.condition); thenStatement.accept(this); if (elseStatement != null) { @@ -3305,7 +3314,12 @@ class _AstVisitor extends GeneralizingAstVisitor { if (expression is SimpleIdentifier) { var element = expression.staticElement; if (element is VariableElement) { - flow.isExpression_end(node, element, typeAnnotation.type); + flow.isExpression_end( + node, + element, + node.notOperator != null, + typeAnnotation.type, + ); } } } @@ -3317,7 +3331,7 @@ class _AstVisitor extends GeneralizingAstVisitor { var operator = node.operator.type; if (operator == TokenType.BANG) { operand.accept(this); - flow.logicalNot_end(node); + flow.logicalNot_end(node, node.operand); } else { operand.accept(this); } @@ -3385,7 +3399,7 @@ class _AstVisitor extends GeneralizingAstVisitor { var member = members[i]; flow.switchStatement_beginCase( - member.labels.isNotEmpty ? assignedInCases : AssignedVariables.emptySet, + member.labels.isNotEmpty ? assignedInCases : assignedVariables.emptySet, ); member.accept(this); @@ -3460,7 +3474,7 @@ class _AstVisitor extends GeneralizingAstVisitor { flow.whileStatement_conditionBegin(assignedVariables[node]); condition.accept(this); - flow.whileStatement_bodyBegin(node); + flow.whileStatement_bodyBegin(node, node.condition); body.accept(this); flow.whileStatement_end(); @@ -3532,7 +3546,34 @@ class _AstVisitor extends GeneralizingAstVisitor { } } -class _TypeSystemTypeOperations implements TypeOperations { +class _FunctionBodyAccess implements FunctionBodyAccess { + final FunctionBody node; + + _FunctionBodyAccess(this.node); + + @override + bool isPotentiallyMutatedInClosure(VariableElement variable) { + return node.isPotentiallyMutatedInClosure(variable); + } + + @override + bool isPotentiallyMutatedInScope(VariableElement variable) { + return node.isPotentiallyMutatedInScope(variable); + } +} + +class _NodeOperations implements NodeOperations { + @override + Expression unwrapParenthesized(Expression node) { + while (node is ParenthesizedExpression) { + node = (node as ParenthesizedExpression).expression; + } + return node; + } +} + +class _TypeSystemTypeOperations + implements TypeOperations { final TypeSystem typeSystem; _TypeSystemTypeOperations(this.typeSystem); @@ -3546,4 +3587,9 @@ class _TypeSystemTypeOperations implements TypeOperations { bool isSubtypeOf(DartType leftType, DartType rightType) { return typeSystem.isSubtypeOf(leftType, rightType); } + + @override + bool isLocalVariable(VariableElement element) { + return element is LocalVariableElement; + } } diff --git a/pkg/nnbd_migration/lib/src/decorated_type_operations.dart b/pkg/nnbd_migration/lib/src/decorated_type_operations.dart index b1e3fa8d47f..0b7a04cbdc1 100644 --- a/pkg/nnbd_migration/lib/src/decorated_type_operations.dart +++ b/pkg/nnbd_migration/lib/src/decorated_type_operations.dart @@ -9,7 +9,8 @@ import 'package:nnbd_migration/src/decorated_type.dart'; import 'package:nnbd_migration/src/node_builder.dart'; /// [TypeOperations] that works with [DecoratedType]s. -class DecoratedTypeOperations implements TypeOperations { +class DecoratedTypeOperations + implements TypeOperations { final TypeSystem _typeSystem; final VariableRepository _variableRepository; @@ -20,6 +21,11 @@ class DecoratedTypeOperations implements TypeOperations { return _variableRepository.decoratedElementType(element); } + @override + bool isLocalVariable(VariableElement element) { + return element is LocalVariableElement; + } + @override bool isSubtypeOf(DecoratedType leftType, DecoratedType rightType) { return _typeSystem.isSubtypeOf(leftType.type, rightType.type);