Restore state safely in migration visitors.

When the migration tool is run in permissive mode, its visitors try to
catch exceptions and continue running.  This means that when the
visitors make temporary state changes before recursive calls, they
have to restore the state inside `finally` blocks.

This should hopefully result in slightly saner behavior when running
the migration tool in permissive mode.

Change-Id: I46e07684526314befad860c4e065ec2c7f6d8b11
Reviewed-on: https://dart-review.googlesource.com/c/93127
Auto-Submit: Paul Berry <paulberry@google.com>
Reviewed-by: Dan Rubel <danrubel@google.com>
This commit is contained in:
Paul Berry 2019-02-14 12:58:09 +00:00 committed by commit-bot@chromium.org
parent 765e6338a8
commit dfda06061a
2 changed files with 50 additions and 29 deletions

View file

@ -240,8 +240,11 @@ class ConstraintGatherer extends GeneralizingAstVisitor<DecoratedType> {
assert(_currentFunctionType == null);
_currentFunctionType =
_variables.decoratedElementType(node.declaredElement);
node.functionExpression.body.accept(this);
_currentFunctionType = null;
try {
node.functionExpression.body.accept(this);
} finally {
_currentFunctionType = null;
}
return null;
}
@ -265,16 +268,22 @@ class ConstraintGatherer extends GeneralizingAstVisitor<DecoratedType> {
if (trueGuard != null) {
_guards.add(trueGuard);
}
node.thenStatement.accept(this);
if (trueGuard != null) {
_guards.removeLast();
try {
node.thenStatement.accept(this);
} finally {
if (trueGuard != null) {
_guards.removeLast();
}
}
if (falseGuard != null) {
_guards.add(falseGuard);
}
node.elseStatement?.accept(this);
if (falseGuard != null) {
_guards.removeLast();
try {
node.elseStatement?.accept(this);
} finally {
if (falseGuard != null) {
_guards.removeLast();
}
}
return null;
}
@ -290,8 +299,11 @@ class ConstraintGatherer extends GeneralizingAstVisitor<DecoratedType> {
assert(_currentFunctionType == null);
_currentFunctionType =
_variables.decoratedElementType(node.declaredElement);
node.body.accept(this);
_currentFunctionType = null;
try {
node.body.accept(this);
} finally {
_currentFunctionType = null;
}
return null;
}
@ -407,23 +419,26 @@ class ConstraintGatherer extends GeneralizingAstVisitor<DecoratedType> {
Expression expression) {
if (sourceType.nullable != null) {
_guards.add(sourceType.nullable);
CheckExpression checkNotNull;
if (expression != null) {
checkNotNull = CheckExpression(expression);
_variables.recordExpressionChecks(
expression, ExpressionChecks(_source, checkNotNull));
}
// nullable_src => nullable_dst | check_expr
_recordFact(ConstraintVariable.or(
_constraints, destinationType.nullable, checkNotNull));
if (checkNotNull != null) {
// nullable_src & nonNullIntent_dst => check_expr
var nonNullIntent = destinationType.nonNullIntent;
if (nonNullIntent != null) {
_recordConstraint(nonNullIntent, checkNotNull);
try {
CheckExpression checkNotNull;
if (expression != null) {
checkNotNull = CheckExpression(expression);
_variables.recordExpressionChecks(
expression, ExpressionChecks(_source, checkNotNull));
}
// nullable_src => nullable_dst | check_expr
_recordFact(ConstraintVariable.or(
_constraints, destinationType.nullable, checkNotNull));
if (checkNotNull != null) {
// nullable_src & nonNullIntent_dst => check_expr
var nonNullIntent = destinationType.nonNullIntent;
if (nonNullIntent != null) {
_recordConstraint(nonNullIntent, checkNotNull);
}
}
} finally {
_guards.removeLast();
}
_guards.removeLast();
}
// 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
@ -508,8 +523,11 @@ class ConstraintGatherer extends GeneralizingAstVisitor<DecoratedType> {
void _recordConstraint(
ConstraintVariable condition, ConstraintVariable consequence) {
_guards.add(condition);
_recordFact(consequence);
_guards.removeLast();
try {
_recordFact(consequence);
} finally {
_guards.removeLast();
}
}
/// Records a constraint having [consequence] as its right hand side. Any

View file

@ -172,8 +172,11 @@ class ConstraintVariableGatherer extends GeneralizingAstVisitor<DecoratedType> {
namedParameters: {},
namedParameterOptionalVariables: {});
_currentFunctionType = functionType;
parameters.accept(this);
_currentFunctionType = previousFunctionType;
try {
parameters.accept(this);
} finally {
_currentFunctionType = previousFunctionType;
}
_variables.recordDecoratedElementType(declaredElement, functionType);
}
}