Skip to content

Commit

Permalink
[flow] Issue 1721 - Allow better promotions for final variables.
Browse files Browse the repository at this point in the history
Promotions should happen for final variables because they are assigned and won't be re-assigned by the time they're evaluated. In a conservative join, promotions should not be cancelled for final variables.

Language tests made in https://dart-review.googlesource.com/c/sdk/+/390340.

Bug: dart-lang/language#1721
Change-Id: I7bb577a694ddb5572a28884de70bd8c5b68e3c25
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/390803
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
  • Loading branch information
kallentu authored and Commit Queue committed Oct 22, 2024
1 parent dacf5bc commit 47d5502
Show file tree
Hide file tree
Showing 18 changed files with 856 additions and 0 deletions.
17 changes: 17 additions & 0 deletions pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2313,6 +2313,11 @@ class FlowModel<Type extends Object> {
PromotionModel<Type>? info =
result.promotionInfo?.get(helper, variableKey);
if (info == null) continue;

// We don't need to discard promotions for final variables. They are
// guaranteed to be already assigned and won't be assigned again.
if (helper.isFinal(variableKey)) continue;

PromotionModel<Type> newInfo =
info.discardPromotionsAndMarkNotUnassigned();
if (!identical(info, newInfo)) {
Expand Down Expand Up @@ -2823,6 +2828,10 @@ mixin FlowModelHelper<Type extends Object> {
/// subtyping.
@visibleForTesting
FlowAnalysisTypeOperations<Type> get typeOperations;

/// Whether the variable of [variableKey] was declared with the `final`
/// modifier and the `inference-update-4` feature flag is enabled.
bool isFinal(int variableKey);
}

/// Documentation links that might be presented to the user to accompany a "why
Expand Down Expand Up @@ -4993,6 +5002,14 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
}
}

@override
bool isFinal(int variableKey) {
if (!inferenceUpdate4Enabled) return false;
Variable? variable = promotionKeyStore.variableForKey(variableKey);
if (variable != null && operations.isFinal(variable)) return true;
return false;
}

@override
bool isUnassigned(Variable variable) {
return _current.promotionInfo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ library;
/// representation of variables and types.
abstract interface class FlowAnalysisOperations<Variable extends Object,
Type extends Object> implements FlowAnalysisTypeOperations<Type> {
/// Whether the given [variable] was declared with the `final` modifier.
bool isFinal(Variable variable);

/// Determines whether the given property can be promoted.
///
/// [property] will correspond to a `propertyMember` value passed to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ class FlowAnalysisTestHarness extends Harness
@override
FlowAnalysisOperations<Var, SharedTypeView<Type>> get typeOperations =>
typeAnalyzer.operations;

@override
bool isFinal(int variableKey) {
Var? variable = promotionKeyStore.variableForKey(variableKey);
if (variable != null && operations.isFinal(variable)) return true;
return false;
}
}

/// Helper class allowing tests to examine the values of variables' SSA nodes.
Expand Down
93 changes: 93 additions & 0 deletions pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,99 @@ main() {
]);
});

test(
'functionExpression_begin() cancels promotions of final vars'
' with inference-update-4 disabled', () {
// See test for "functionExpression_begin() preserves promotions of final
// variables" for enabled behavior.
var x = Var('x', isFinal: true);
h.disableInferenceUpdate4();
h.run([
declare(x, type: 'num'),
if_(expr('bool'), [
x.write(expr('int')),
], [
x.write(expr('double')),
]),
if_(x.is_('int'), [
localFunction([
checkNotPromoted(x),
]),
], [
localFunction([
checkNotPromoted(x),
]),
]),
]);
});

test('functionExpression_begin() cancels promotions of non-final vars', () {
// num x;
// if (<bool>) {
// x = <int>;
// } else {
// x = <double>;
// }
// if (x is int) {
// () => x is not promoted
// } else {
// () => x is not promoted
// }

var x = Var('x');
h.run([
declare(x, type: 'num'),
if_(expr('bool'), [
x.write(expr('int')),
], [
x.write(expr('double')),
]),
if_(x.is_('int'), [
localFunction([
checkNotPromoted(x),
]),
], [
localFunction([
checkNotPromoted(x),
]),
]),
]);
});

test('functionExpression_begin() preserves promotions of final variables',
() {
// final num x;
// if (<bool>) {
// x = <int>;
// } else {
// x = <double>;
// }
// if (x is int) {
// () => x is promoted to int
// } else {
// () => x is not promoted
// }

var x = Var('x', isFinal: true);
h.run([
declare(x, type: 'num'),
if_(expr('bool'), [
x.write(expr('int')),
], [
x.write(expr('double')),
]),
if_(x.is_('int'), [
localFunction([
checkPromoted(x, 'int'),
]),
], [
localFunction([
checkNotPromoted(x),
]),
]),
]);
});

test('functionExpression_begin() preserves promotions of initialized vars',
() {
var x = Var('x');
Expand Down
5 changes: 5 additions & 0 deletions pkg/_fe_analyzer_shared/test/mini_ast.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2963,6 +2963,11 @@ class MiniAstOperations
return false;
}

@override
bool isFinal(Var variable) {
return variable.isFinal;
}

@override
bool isInterfaceType(SharedTypeView<Type> type) {
Type unwrappedType = type.unwrapTypeView();
Expand Down
5 changes: 5 additions & 0 deletions pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,11 @@ class TypeSystemOperations
unwrappedType.element is ExtensionTypeElement;
}

@override
bool isFinal(PromotableElement variable) {
return variable.isFinal;
}

@override
bool isInterfaceType(SharedTypeView<DartType> type) {
DartType unwrappedType = type.unwrapTypeView();
Expand Down
Loading

0 comments on commit 47d5502

Please sign in to comment.