mirror of
https://github.com/dart-lang/sdk
synced 2024-10-14 18:49:37 +00:00
Front end: type infer all record literal fields before hoisting.
Previously, if a record literal had named fields, type inference of the literal fields would happen as part of the process of hoisting. This was a problem because the hoisting process requires visiting the fields in reverse order, so as a result, the order in which subexpressions were type inferred was the reverse of the order in which they would be executed, leading to unsound type promotion behavior. Fixes #55914. Bug: https://github.com/dart-lang/sdk/issues/55914 Change-Id: I2d8930e0e1d7579d065bfb850aa7472d28a8012c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/369761 Reviewed-by: Chloe Stefantsova <cstefantsova@google.com> Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
parent
8e00ebc689
commit
c007d3bc0c
|
@ -9152,20 +9152,58 @@ class InferenceVisitorImpl extends InferenceVisitorBase
|
|||
positional[index] = expressionResult.expression;
|
||||
}
|
||||
} else {
|
||||
List<String> sortedNames = namedElements.keys.toList()..sort();
|
||||
|
||||
positionalTypes =
|
||||
new List<DartType>.filled(positional.length, const UnknownType());
|
||||
Map<String, DartType> namedElementTypes = {};
|
||||
|
||||
// Index into [positional] of the positional element we find next.
|
||||
int positionalIndex = 0;
|
||||
|
||||
for (int index = 0; index < originalElementOrder.length; index++) {
|
||||
Object element = originalElementOrder[index];
|
||||
if (element is NamedExpression) {
|
||||
DartType contextType =
|
||||
namedTypeContexts?[element.name] ?? const UnknownType();
|
||||
ExpressionInferenceResult expressionResult =
|
||||
inferExpression(element.value, contextType);
|
||||
if (contextType is! UnknownType) {
|
||||
expressionResult = coerceExpressionForAssignment(
|
||||
contextType, expressionResult,
|
||||
treeNodeForTesting: node) ??
|
||||
expressionResult;
|
||||
}
|
||||
Expression expression = expressionResult.expression;
|
||||
DartType type = expressionResult.postCoercionType ??
|
||||
expressionResult.inferredType;
|
||||
element.value = expression..parent = element;
|
||||
namedElementTypes[element.name] = type;
|
||||
} else {
|
||||
DartType contextType =
|
||||
positionalTypeContexts?[positionalIndex] ?? const UnknownType();
|
||||
ExpressionInferenceResult expressionResult =
|
||||
inferExpression(element as Expression, contextType);
|
||||
if (contextType is! UnknownType) {
|
||||
expressionResult = coerceExpressionForAssignment(
|
||||
contextType, expressionResult,
|
||||
treeNodeForTesting: node) ??
|
||||
expressionResult;
|
||||
}
|
||||
Expression expression = expressionResult.expression;
|
||||
DartType type = expressionResult.postCoercionType ??
|
||||
expressionResult.inferredType;
|
||||
positional[positionalIndex] = expression;
|
||||
positionalTypes[positionalIndex] = type;
|
||||
positionalIndex++;
|
||||
}
|
||||
}
|
||||
|
||||
List<String> sortedNames = namedElements.keys.toList()..sort();
|
||||
|
||||
// Index into [sortedNames] of the named element we expected to find
|
||||
// next, for the named elements to be sorted. This also used to detect
|
||||
// when all named elements have been seen, even when they are not sorted.
|
||||
int nameIndex = sortedNames.length - 1;
|
||||
|
||||
// Index into [positional] of the positional element we find next.
|
||||
int positionalIndex = positional.length - 1;
|
||||
|
||||
// For const literals we don't hoist to avoid using let variables in
|
||||
// inside constants. Since the elements of the literal must be constant
|
||||
// themselves, we know that there is no side effects of performing
|
||||
|
@ -9183,22 +9221,12 @@ class InferenceVisitorImpl extends InferenceVisitorBase
|
|||
// expressions we need to hoist. When we observe an element out of order,
|
||||
// either positional after named or unsorted named, all preceding
|
||||
// elements must be hoisted to retain the original evaluation order.
|
||||
positionalIndex--;
|
||||
for (int index = originalElementOrder.length - 1; index >= 0; index--) {
|
||||
Object element = originalElementOrder[index];
|
||||
if (element is NamedExpression) {
|
||||
DartType contextType =
|
||||
namedTypeContexts?[element.name] ?? const UnknownType();
|
||||
ExpressionInferenceResult expressionResult =
|
||||
inferExpression(element.value, contextType);
|
||||
if (contextType is! UnknownType) {
|
||||
expressionResult = coerceExpressionForAssignment(
|
||||
contextType, expressionResult,
|
||||
treeNodeForTesting: node) ??
|
||||
expressionResult;
|
||||
}
|
||||
Expression expression = expressionResult.expression;
|
||||
DartType type = expressionResult.postCoercionType ??
|
||||
expressionResult.inferredType;
|
||||
Expression expression = element.value;
|
||||
DartType type = namedElementTypes[element.name]!;
|
||||
// TODO(johnniwinther): Should we use [isPureExpression] as is, make
|
||||
// it include (simple) literals, or add a new predicate?
|
||||
if (needsHoisting && !isPureExpression(expression)) {
|
||||
|
@ -9208,10 +9236,7 @@ class InferenceVisitorImpl extends InferenceVisitorBase
|
|||
hoistedExpressions ??= [];
|
||||
hoistedExpressions.add(variable);
|
||||
element.value = createVariableGet(variable)..parent = element;
|
||||
} else {
|
||||
element.value = expression..parent = element;
|
||||
}
|
||||
namedElementTypes[element.name] = type;
|
||||
if (!namedNeedsSorting && element.name != sortedNames[nameIndex]) {
|
||||
// Named elements are not sorted, so we need to hoist and sort them.
|
||||
namedNeedsSorting = true;
|
||||
|
@ -9219,19 +9244,8 @@ class InferenceVisitorImpl extends InferenceVisitorBase
|
|||
}
|
||||
nameIndex--;
|
||||
} else {
|
||||
DartType contextType =
|
||||
positionalTypeContexts?[positionalIndex] ?? const UnknownType();
|
||||
ExpressionInferenceResult expressionResult =
|
||||
inferExpression(element as Expression, contextType);
|
||||
if (contextType is! UnknownType) {
|
||||
expressionResult = coerceExpressionForAssignment(
|
||||
contextType, expressionResult,
|
||||
treeNodeForTesting: node) ??
|
||||
expressionResult;
|
||||
}
|
||||
Expression expression = expressionResult.expression;
|
||||
DartType type = expressionResult.postCoercionType ??
|
||||
expressionResult.inferredType;
|
||||
Expression expression = positional[positionalIndex];
|
||||
DartType type = positionalTypes[positionalIndex];
|
||||
// TODO(johnniwinther): Should we use [isPureExpression] as is, make
|
||||
// it include (simple) literals, or add a new predicate?
|
||||
if (needsHoisting && !isPureExpression(expression)) {
|
||||
|
@ -9241,15 +9255,11 @@ class InferenceVisitorImpl extends InferenceVisitorBase
|
|||
hoistedExpressions ??= [];
|
||||
hoistedExpressions.add(variable);
|
||||
positional[positionalIndex] = createVariableGet(variable);
|
||||
} else {
|
||||
positional[positionalIndex] = expression;
|
||||
if (nameIndex >= 0) {
|
||||
} else if (nameIndex >= 0) {
|
||||
// We have not seen all named elements yet, so we must hoist the
|
||||
// remaining named elements and the preceding positional elements.
|
||||
needsHoisting = enableHoisting;
|
||||
}
|
||||
}
|
||||
positionalTypes[positionalIndex] = type;
|
||||
positionalIndex--;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,130 @@
|
|||
// Copyright (c) 2024, 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.
|
||||
|
||||
/// Test that if a record literal contains named fields, then all of the record
|
||||
/// literal's fields are type inferred in the order in which they are written,
|
||||
/// regardless of the sort order of the field names, and regardless of the
|
||||
/// relative ordering of named and unnamed fields.
|
||||
///
|
||||
/// See https://github.com/dart-lang/sdk/issues/55914
|
||||
|
||||
import '../static_type_helper.dart';
|
||||
|
||||
({Object a, Object b, Object c}) testSortedNamedFields(Object o1, Object o2) {
|
||||
// Check that `a` is type inferred before `b` by promoting `o1` in `a` and
|
||||
// checking its type in `b`. Check that `b` is type inferred before `c` by
|
||||
// promoting `o2` in `b` and checking its type in `c`.
|
||||
return (
|
||||
a: o1 as int,
|
||||
b: [o1..expectStaticType<Exactly<int>>(), o2 as int],
|
||||
c: o2..expectStaticType<Exactly<int>>()
|
||||
);
|
||||
}
|
||||
|
||||
({Object a, Object b, Object c}) testReversedNamedFields(Object o1, Object o2) {
|
||||
// Check that `c` is type inferred before `b` by promoting `o1` in `c` and
|
||||
// checking its type in `b`. Check that `b` is type inferred before `a` by
|
||||
// promoting `o2` in `b` and checking its type in `a`.
|
||||
return (
|
||||
c: o1 as int,
|
||||
b: [o1..expectStaticType<Exactly<int>>(), o2 as int],
|
||||
a: o2..expectStaticType<Exactly<int>>()
|
||||
);
|
||||
}
|
||||
|
||||
(Object, {Object a, Object b}) testSortedNamedFieldsAfterPositional(
|
||||
Object o1, Object o2) {
|
||||
// Check that `$1` is type inferred before `a` by promoting `o1` in `$1` and
|
||||
// checking its type in `a`. Check that `a` is type inferred before `b` by
|
||||
// promoting `o2` in `a` and checking its type in `b`.
|
||||
return (
|
||||
o1 as int,
|
||||
a: [o1..expectStaticType<Exactly<int>>(), o2 as int],
|
||||
b: o2..expectStaticType<Exactly<int>>()
|
||||
);
|
||||
}
|
||||
|
||||
(Object, {Object a, Object b}) testReversedNamedFieldsAfterPositional(
|
||||
Object o1, Object o2) {
|
||||
// Check that `$1` is type inferred before `b` by promoting `o1` in `$1` and
|
||||
// checking its type in `b`. Check that `b` is type inferred before `a` by
|
||||
// promoting `o2` in `b` and checking its type in `a`.
|
||||
return (
|
||||
o1 as int,
|
||||
b: [o1..expectStaticType<Exactly<int>>(), o2 as int],
|
||||
a: o2..expectStaticType<Exactly<int>>()
|
||||
);
|
||||
}
|
||||
|
||||
(Object, {Object a, Object b}) testSortedNamedFieldsBeforePositional(
|
||||
Object o1, Object o2) {
|
||||
// Check that `a` is type inferred before `b` by promoting `o1` in `a` and
|
||||
// checking its type in `b`. Check that `b` is type inferred before `$1` by
|
||||
// promoting `o2` in `b` and checking its type in `$1`.
|
||||
return (
|
||||
a: o1 as int,
|
||||
b: [o1..expectStaticType<Exactly<int>>(), o2 as int],
|
||||
o2..expectStaticType<Exactly<int>>()
|
||||
);
|
||||
}
|
||||
|
||||
(Object, {Object a, Object b}) testReversedNamedFieldsBeforePositional(
|
||||
Object o1, Object o2) {
|
||||
// Check that `b` is type inferred before `a` by promoting `o1` in `b` and
|
||||
// checking its type in `a`. Check that `a` is type inferred before `$1` by
|
||||
// promoting `o2` in `a` and checking its type in `$1`.
|
||||
return (
|
||||
b: o1 as int,
|
||||
a: [o1..expectStaticType<Exactly<int>>(), o2 as int],
|
||||
o2..expectStaticType<Exactly<int>>()
|
||||
);
|
||||
}
|
||||
|
||||
(Object, Object, {Object a}) testSingleNamedFieldAfterPositionals(
|
||||
Object o1, Object o2) {
|
||||
// Check that `$1` is type inferred before `$2` by promoting `o1` in `$1` and
|
||||
// checking its type in `$2`. Check that `$2` is type inferred before `a` by
|
||||
// promoting `o2` in `$2` and checking its type in `a`.
|
||||
return (
|
||||
o1 as int,
|
||||
[o1..expectStaticType<Exactly<int>>(), o2 as int],
|
||||
a: o2..expectStaticType<Exactly<int>>()
|
||||
);
|
||||
}
|
||||
|
||||
(Object, Object, {Object a}) testSingleNamedFieldBetweenPositionals(
|
||||
Object o1, Object o2) {
|
||||
// Check that `$1` is type inferred before `a` by promoting `o1` in `$1` and
|
||||
// checking its type in `a`. Check that `a` is type inferred before `$2` by
|
||||
// promoting `o2` in `a` and checking its type in `$2`.
|
||||
return (
|
||||
o1 as int,
|
||||
a: [o1..expectStaticType<Exactly<int>>(), o2 as int],
|
||||
o2..expectStaticType<Exactly<int>>()
|
||||
);
|
||||
}
|
||||
|
||||
(Object, Object, {Object a}) testSingleNamedFieldBeforePositionals(
|
||||
Object o1, Object o2) {
|
||||
// Check that `a` is type inferred before `$1` by promoting `o1` in `a` and
|
||||
// checking its type in `$1`. Check that `$1` is type inferred before `$2` by
|
||||
// promoting `o2` in `$1` and checking its type in `$2`.
|
||||
return (
|
||||
a: o1 as int,
|
||||
[o1..expectStaticType<Exactly<int>>(), o2 as int],
|
||||
o2..expectStaticType<Exactly<int>>()
|
||||
);
|
||||
}
|
||||
|
||||
main() {
|
||||
testSortedNamedFields(0, 1);
|
||||
testReversedNamedFields(0, 1);
|
||||
testSortedNamedFieldsAfterPositional(0, 1);
|
||||
testReversedNamedFieldsAfterPositional(0, 1);
|
||||
testSortedNamedFieldsBeforePositional(0, 1);
|
||||
testReversedNamedFieldsBeforePositional(0, 1);
|
||||
testSingleNamedFieldAfterPositionals(0, 1);
|
||||
testSingleNamedFieldBetweenPositionals(0, 1);
|
||||
testSingleNamedFieldBeforePositionals(0, 1);
|
||||
}
|
Loading…
Reference in a new issue