Skip to content

Commit

Permalink
[flow] Part 3 Issue 3658 Avoid writing promotion information for post…
Browse files Browse the repository at this point in the history
…fix inc/dec expressions.

Postfix increment and decrement expressions should not be saving any promotion information.

An example of where saving the promotion information after the write is unsafe is:
```
class A {
  A operator +(int i) {
    return new B();
  }
}

class B extends A {}

main() {
  A x = A();
  if ((x++) is B) {
    // x should not be B
  }
}
```

This change is only for the analyzer because the CFE does something different (converts the postfix increment/decrement into a let expression).

Bug: dart-lang/language#3658
Change-Id: Ic22f69bf79da66965908ade80bdf70399f0bcaa3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391494
Reviewed-by: Paul Berry <[email protected]>
Commit-Queue: Kallen Tu <[email protected]>
  • Loading branch information
kallentu authored and Commit Queue committed Oct 31, 2024
1 parent 507d40f commit 00e4c92
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 4 deletions.
26 changes: 24 additions & 2 deletions pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,10 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
/// state that was saved by [pushSubpattern].
void popSubpattern();

/// Call this method when writing to the [variable] with type [writtenType] in
/// a postfix increment or decrement operation.
void postIncDec(Node node, Variable variable, Type writtenType);

/// Retrieves the type that a property named [propertyName] is promoted to, if
/// the property is currently promoted. Otherwise returns `null`.
///
Expand Down Expand Up @@ -1818,6 +1822,12 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
_wrap('popSubpattern()', () => _wrapped.popSubpattern());
}

@override
void postIncDec(Node node, Variable variable, Type writtenType) {
_wrap(
'postIncDec()', () => _wrapped.postIncDec(node, variable, writtenType));
}

@override
Type? promotedPropertyType(PropertyTarget<Expression> target,
String propertyName, Object? propertyMember, Type unpromotedType) {
Expand Down Expand Up @@ -5296,6 +5306,11 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
assert(context is _PatternContext<Type>);
}

@override
void postIncDec(Node node, Variable variable, Type writtenType) {
_write(node, variable, writtenType, null, isPostfixIncDec: true);
}

@override
Type? promotedPropertyType(PropertyTarget<Expression> target,
String propertyName, Object? propertyMember, Type unpromotedType) {
Expand Down Expand Up @@ -6274,8 +6289,12 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,

/// Common logic for handling writes to variables, whether they occur as part
/// of an ordinary assignment or a pattern assignment.
///
/// If [isPostfixIncDec] is `true`, the [node] is a postfix expression and we
/// won't store information about [variable].
void _write(Node node, Variable variable, Type writtenType,
ExpressionInfo<Type>? expressionInfo) {
ExpressionInfo<Type>? expressionInfo,
{bool isPostfixIncDec = false}) {
Type unpromotedType = operations.variableType(variable);
int variableKey = promotionKeyStore.keyForVariable(variable);
SsaNode<Type> newSsaNode = new SsaNode<Type>(
Expand All @@ -6292,7 +6311,7 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
unpromotedType: unpromotedType);

// Update the type of the variable for looking up the write expression.
if (inferenceUpdate4Enabled && node is Expression) {
if (inferenceUpdate4Enabled && node is Expression && !isPostfixIncDec) {
_Reference<Type> reference = _variableReference(
variableKey,
unpromotedType,
Expand Down Expand Up @@ -6891,6 +6910,9 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
@override
void popSubpattern() {}

@override
void postIncDec(Node node, Variable variable, Type writtenType) {}

@override
Type? promotedPropertyType(PropertyTarget<Expression> target,
String propertyName, Object? propertyMember, Type unpromotedType) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.

import 'package:_fe_analyzer_shared/src/types/shared_type.dart';
import 'package:analyzer/dart/analysis/features.dart';
import 'package:analyzer/dart/ast/token.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
Expand Down Expand Up @@ -176,8 +177,17 @@ class PostfixExpressionResolver {
if (operand is SimpleIdentifier) {
var element = operand.staticElement;
if (element is PromotableElement) {
_resolver.flowAnalysis.flow
?.write(node, element, SharedTypeView(operatorReturnType), null);
if (_resolver.definingLibrary.featureSet
.isEnabled(Feature.inference_update_4)) {
_resolver.flowAnalysis.flow?.postIncDec(
node,
element,
SharedTypeView(operatorReturnType),
);
} else {
_resolver.flowAnalysis.flow?.write(
node, element, SharedTypeView(operatorReturnType), null);
}
}
}
node.recordStaticType(receiverType, resolver: _resolver);
Expand Down
33 changes: 33 additions & 0 deletions pkg/analyzer/test/src/dart/resolution/postfix_expression_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// 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/analysis/features.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/src/dart/error/syntactic_errors.dart';
import 'package:analyzer/src/error/codes.dart';
Expand All @@ -11,10 +12,42 @@ import 'context_collection_resolution.dart';

main() {
defineReflectiveSuite(() {
defineReflectiveTests(InferenceUpdate4Test);
defineReflectiveTests(PostfixExpressionResolutionTest);
});
}

@reflectiveTest
class InferenceUpdate4Test extends PubPackageResolutionTest {
@override
List<String> get experiments {
return [
...super.experiments,
Feature.inference_update_4.enableString,
];
}

test_isExpression_notPromoted() async {
await assertNoErrorsInCode('''
f() {
num x = 2;
if ((x++) is int) {
x;
}
}
''');
var nonPromotedIdentifier = findNode.simple('x;');
assertResolvedNodeText(nonPromotedIdentifier, r'''
SimpleIdentifier
token: x
staticElement: x@12
element: x@12
staticType: num
''');
assertType(nonPromotedIdentifier, 'num');
}
}

@reflectiveTest
class PostfixExpressionResolutionTest extends PubPackageResolutionTest {
test_dec_simpleIdentifier_parameter_int() async {
Expand Down

0 comments on commit 00e4c92

Please sign in to comment.