Skip to content

Commit 2eba00f

Browse files
chloestefantsovaCommit Queue
authored and
Commit Queue
committed
[cfe] Split ensureAssignableResult into coersion and error reporting
It allows to use coersion without error reporting in the inference of record literals, which makes the difference when the record patterns are involved. Closes #51587 Part of #49749 Change-Id: I64ea7e82eb3bc14d4410a47c6ed6ef278e092496 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/290529 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Chloe Stefantsova <[email protected]>
1 parent 6b4878b commit 2eba00f

12 files changed

+399
-22
lines changed

pkg/front_end/lib/src/fasta/type_inference/inference_visitor.dart

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9146,7 +9146,8 @@ class InferenceVisitorImpl extends InferenceVisitorBase
91469146
inferExpression(expression, contextType);
91479147
if (contextType is! UnknownType) {
91489148
expressionResult =
9149-
ensureAssignableResult(contextType, expressionResult);
9149+
coerceExpressionForAssignment(contextType, expressionResult) ??
9150+
expressionResult;
91509151
}
91519152

91529153
positionalTypes.add(
@@ -9194,7 +9195,8 @@ class InferenceVisitorImpl extends InferenceVisitorBase
91949195
inferExpression(element.value, contextType);
91959196
if (contextType is! UnknownType) {
91969197
expressionResult =
9197-
ensureAssignableResult(contextType, expressionResult);
9198+
coerceExpressionForAssignment(contextType, expressionResult) ??
9199+
expressionResult;
91989200
}
91999201
Expression expression = expressionResult.expression;
92009202
DartType type = expressionResult.postCoercionType ??
@@ -9225,7 +9227,8 @@ class InferenceVisitorImpl extends InferenceVisitorBase
92259227
inferExpression(element as Expression, contextType);
92269228
if (contextType is! UnknownType) {
92279229
expressionResult =
9228-
ensureAssignableResult(contextType, expressionResult);
9230+
coerceExpressionForAssignment(contextType, expressionResult) ??
9231+
expressionResult;
92299232
}
92309233
Expression expression = expressionResult.expression;
92319234
DartType type = expressionResult.postCoercionType ??

pkg/front_end/lib/src/fasta/type_inference/inference_visitor_base.dart

Lines changed: 146 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -445,16 +445,102 @@ abstract class InferenceVisitorBase implements InferenceVisitor {
445445
.expression;
446446
}
447447

448-
/// Same as [ensureAssignable], but accepts an [ExpressionInferenceResult]
449-
/// rather than an expression and a type separately. If no change is made,
450-
/// [inferenceResult] is returned unchanged.
451-
ExpressionInferenceResult ensureAssignableResult(
448+
/// Coerces expression ensuring its assignability to [contextType]
449+
///
450+
/// If the expression is assignable without coercion, [inferenceResult]
451+
/// is returned unchanged. If no coercion is possible for the given types,
452+
/// `null` is returned.
453+
ExpressionInferenceResult? coerceExpressionForAssignment(
452454
DartType contextType, ExpressionInferenceResult inferenceResult,
453455
{int? fileOffset,
454456
DartType? declaredContextType,
455457
DartType? runtimeCheckedType,
456458
bool isVoidAllowed = false,
457-
bool coerceExpression = true,
459+
bool coerceExpression = true}) {
460+
// ignore: unnecessary_null_comparison
461+
assert(contextType != null);
462+
463+
fileOffset ??= inferenceResult.expression.fileOffset;
464+
contextType = computeGreatestClosure(contextType);
465+
466+
DartType initialContextType = runtimeCheckedType ?? contextType;
467+
468+
Template<Message Function(DartType, DartType, bool)>?
469+
preciseTypeErrorTemplate =
470+
_getPreciseTypeErrorTemplate(inferenceResult.expression);
471+
AssignabilityResult assignabilityResult = _computeAssignabilityKind(
472+
contextType, inferenceResult.inferredType,
473+
isNonNullableByDefault: isNonNullableByDefault,
474+
isVoidAllowed: isVoidAllowed,
475+
isExpressionTypePrecise: preciseTypeErrorTemplate != null,
476+
coerceExpression: coerceExpression);
477+
478+
if (assignabilityResult.needsTearOff) {
479+
TypedTearoff typedTearoff = _tearOffCall(inferenceResult.expression,
480+
inferenceResult.inferredType as InterfaceType, fileOffset);
481+
inferenceResult = new ExpressionInferenceResult(
482+
typedTearoff.tearoffType, typedTearoff.tearoff);
483+
}
484+
if (assignabilityResult.implicitInstantiation != null) {
485+
inferenceResult = _applyImplicitInstantiation(
486+
assignabilityResult.implicitInstantiation,
487+
inferenceResult.inferredType,
488+
inferenceResult.expression);
489+
}
490+
491+
DartType expressionType = inferenceResult.inferredType;
492+
Expression expression = inferenceResult.expression;
493+
switch (assignabilityResult.kind) {
494+
case AssignabilityKind.assignable:
495+
return inferenceResult;
496+
case AssignabilityKind.assignableCast:
497+
// Insert an implicit downcast.
498+
Expression asExpression =
499+
new AsExpression(expression, initialContextType)
500+
..isTypeError = true
501+
..isForNonNullableByDefault = isNonNullableByDefault
502+
..isForDynamic = expressionType is DynamicType
503+
..fileOffset = fileOffset;
504+
flowAnalysis.forwardExpression(asExpression, expression);
505+
return new ExpressionInferenceResult(expressionType, asExpression,
506+
postCoercionType: initialContextType);
507+
case AssignabilityKind.unassignable:
508+
// Error: not assignable. Perform error recovery.
509+
return null;
510+
case AssignabilityKind.unassignableVoid:
511+
// Error: not assignable. Perform error recovery.
512+
return null;
513+
case AssignabilityKind.unassignablePrecise:
514+
// The type of the expression is known precisely, so an implicit
515+
// downcast is guaranteed to fail. Insert a compile-time error.
516+
return null;
517+
case AssignabilityKind.unassignableCantTearoff:
518+
return null;
519+
case AssignabilityKind.unassignableNullability:
520+
return null;
521+
default:
522+
return unhandled("${assignabilityResult}", "ensureAssignable",
523+
fileOffset, helper.uri);
524+
}
525+
}
526+
527+
/// Performs assignability checks on an expression
528+
///
529+
/// [inferenceResult.expression] of type [inferenceResult.inferredType] is
530+
/// checked for assignability to [contextType]. The errors are reported on the
531+
/// current library and the expression wrapped in an [InvalidExpression], if
532+
/// needed. If no change is made, [inferenceResult] is returned unchanged.
533+
///
534+
/// If [isCoercionAllowed] is `true`, the assignability check is made
535+
/// accounting for a possible coercion that may adjust the type of the
536+
/// expression.
537+
ExpressionInferenceResult reportAssignabilityErrors(
538+
DartType contextType, ExpressionInferenceResult inferenceResult,
539+
{int? fileOffset,
540+
DartType? declaredContextType,
541+
DartType? runtimeCheckedType,
542+
bool isVoidAllowed = false,
543+
bool isCoercionAllowed = true,
458544
Template<Message Function(DartType, DartType, bool)>? errorTemplate,
459545
Template<Message Function(DartType, DartType, bool)>?
460546
nullabilityErrorTemplate,
@@ -492,8 +578,6 @@ abstract class InferenceVisitorBase implements InferenceVisitor {
492578
fileOffset ??= inferenceResult.expression.fileOffset;
493579
contextType = computeGreatestClosure(contextType);
494580

495-
DartType initialContextType = runtimeCheckedType ?? contextType;
496-
497581
Template<Message Function(DartType, DartType, bool)>?
498582
preciseTypeErrorTemplate =
499583
_getPreciseTypeErrorTemplate(inferenceResult.expression);
@@ -502,7 +586,7 @@ abstract class InferenceVisitorBase implements InferenceVisitor {
502586
isNonNullableByDefault: isNonNullableByDefault,
503587
isVoidAllowed: isVoidAllowed,
504588
isExpressionTypePrecise: preciseTypeErrorTemplate != null,
505-
coerceExpression: coerceExpression);
589+
coerceExpression: isCoercionAllowed);
506590

507591
if (assignabilityResult.needsTearOff) {
508592
TypedTearoff typedTearoff = _tearOffCall(inferenceResult.expression,
@@ -519,20 +603,12 @@ abstract class InferenceVisitorBase implements InferenceVisitor {
519603

520604
DartType expressionType = inferenceResult.inferredType;
521605
Expression expression = inferenceResult.expression;
522-
Expression result;
523-
DartType? postCoercionType;
606+
DartType? postCoercionType = inferenceResult.postCoercionType;
607+
Expression? result;
524608
switch (assignabilityResult.kind) {
525609
case AssignabilityKind.assignable:
526-
result = expression;
527610
break;
528611
case AssignabilityKind.assignableCast:
529-
// Insert an implicit downcast.
530-
result = new AsExpression(expression, initialContextType)
531-
..isTypeError = true
532-
..isForNonNullableByDefault = isNonNullableByDefault
533-
..isForDynamic = expressionType is DynamicType
534-
..fileOffset = fileOffset;
535-
postCoercionType = initialContextType;
536612
break;
537613
case AssignabilityKind.unassignable:
538614
// Error: not assignable. Perform error recovery.
@@ -615,7 +691,7 @@ abstract class InferenceVisitorBase implements InferenceVisitor {
615691
fileOffset, helper.uri);
616692
}
617693

618-
if (!identical(result, expression)) {
694+
if (result != null) {
619695
flowAnalysis.forwardExpression(result, expression);
620696
return new ExpressionInferenceResult(expressionType, result,
621697
postCoercionType: postCoercionType);
@@ -624,6 +700,57 @@ abstract class InferenceVisitorBase implements InferenceVisitor {
624700
}
625701
}
626702

703+
/// Same as [ensureAssignable], but accepts an [ExpressionInferenceResult]
704+
/// rather than an expression and a type separately. If no change is made,
705+
/// [inferenceResult] is returned unchanged.
706+
ExpressionInferenceResult ensureAssignableResult(
707+
DartType contextType, ExpressionInferenceResult inferenceResult,
708+
{int? fileOffset,
709+
DartType? declaredContextType,
710+
DartType? runtimeCheckedType,
711+
bool isVoidAllowed = false,
712+
bool coerceExpression = true,
713+
Template<Message Function(DartType, DartType, bool)>? errorTemplate,
714+
Template<Message Function(DartType, DartType, bool)>?
715+
nullabilityErrorTemplate,
716+
Template<Message Function(DartType, bool)>? nullabilityNullErrorTemplate,
717+
Template<Message Function(DartType, DartType, bool)>?
718+
nullabilityNullTypeErrorTemplate,
719+
Template<Message Function(DartType, DartType, DartType, DartType, bool)>?
720+
nullabilityPartErrorTemplate,
721+
Map<DartType, NonPromotionReason> Function()? whyNotPromoted}) {
722+
// ignore: unnecessary_null_comparison
723+
assert(contextType != null);
724+
725+
if (coerceExpression) {
726+
ExpressionInferenceResult? coercionResult = coerceExpressionForAssignment(
727+
contextType, inferenceResult,
728+
fileOffset: fileOffset,
729+
declaredContextType: declaredContextType,
730+
runtimeCheckedType: runtimeCheckedType,
731+
isVoidAllowed: isVoidAllowed,
732+
coerceExpression: coerceExpression);
733+
if (coercionResult != null) {
734+
return coercionResult;
735+
}
736+
}
737+
738+
inferenceResult = reportAssignabilityErrors(contextType, inferenceResult,
739+
fileOffset: fileOffset,
740+
declaredContextType: declaredContextType,
741+
runtimeCheckedType: runtimeCheckedType,
742+
isVoidAllowed: isVoidAllowed,
743+
isCoercionAllowed: coerceExpression,
744+
errorTemplate: errorTemplate,
745+
nullabilityErrorTemplate: nullabilityErrorTemplate,
746+
nullabilityNullErrorTemplate: nullabilityNullErrorTemplate,
747+
nullabilityNullTypeErrorTemplate: nullabilityNullTypeErrorTemplate,
748+
nullabilityPartErrorTemplate: nullabilityPartErrorTemplate,
749+
whyNotPromoted: whyNotPromoted);
750+
751+
return inferenceResult;
752+
}
753+
627754
Expression _wrapTearoffErrorExpression(Expression expression,
628755
DartType contextType, Template<Message Function(String)> template) {
629756
// ignore: unnecessary_null_comparison

pkg/front_end/test/spell_checking_list_common.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,7 @@ clue
495495
code
496496
coerce
497497
coerced
498+
coerces
498499
coercing
499500
coercion
500501
coincides
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
test1() {
6+
num b = 0;
7+
b as int;
8+
b.isEven;
9+
(b,) = (3.14,);
10+
return b;
11+
}
12+
13+
test2() {
14+
num b = 0;
15+
b as int;
16+
b.isEven;
17+
(b, foo: _) = (3.14, foo: "foo");
18+
return b;
19+
}
20+
21+
test3() {
22+
num b = 0;
23+
b as int;
24+
b.isEven;
25+
(foo: b) = (foo: 3.14);
26+
return b;
27+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
library /*isNonNullableByDefault*/;
2+
import self as self;
3+
import "dart:core" as core;
4+
import "dart:_internal" as _in;
5+
6+
static method test1() → dynamic {
7+
core::num b = 0;
8+
b as{ForNonNullableByDefault} core::int;
9+
b{core::int}.{core::int::isEven}{core::bool};
10+
block {
11+
final synthesized dynamic #0#0 = (3.14);
12+
if(!(let final dynamic #t1 = b = #0#0{(core::double)}.$1{core::double} in true))
13+
throw new _in::ReachabilityError::•();
14+
} =>#0#0;
15+
return b;
16+
}
17+
static method test2() → dynamic {
18+
core::num b = 0;
19+
b as{ForNonNullableByDefault} core::int;
20+
b{core::int}.{core::int::isEven}{core::bool};
21+
block {
22+
final synthesized dynamic #0#0 = (3.14, {foo: "foo"});
23+
if(!((let final dynamic #t2 = b = #0#0{(core::double, {foo: core::String})}.$1{core::double} in true) && true))
24+
throw new _in::ReachabilityError::•();
25+
} =>#0#0;
26+
return b;
27+
}
28+
static method test3() → dynamic {
29+
core::num b = 0;
30+
b as{ForNonNullableByDefault} core::int;
31+
b{core::int}.{core::int::isEven}{core::bool};
32+
block {
33+
final synthesized dynamic #0#0 = ({foo: 3.14});
34+
if(!(let final dynamic #t3 = b = #0#0{({foo: core::double})}.foo{core::double} in true))
35+
throw new _in::ReachabilityError::•();
36+
} =>#0#0;
37+
return b;
38+
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
library /*isNonNullableByDefault*/;
2+
import self as self;
3+
import "dart:core" as core;
4+
import "dart:_internal" as _in;
5+
6+
static method test1() → dynamic {
7+
core::num b = 0;
8+
b as{ForNonNullableByDefault} core::int;
9+
b{core::int}.{core::int::isEven}{core::bool};
10+
block {
11+
final synthesized dynamic #0#0 = (3.14);
12+
if(!(let final core::double #t1 = b = #0#0{(core::double)}.$1{core::double} in true))
13+
throw new _in::ReachabilityError::•();
14+
} =>#0#0;
15+
return b;
16+
}
17+
static method test2() → dynamic {
18+
core::num b = 0;
19+
b as{ForNonNullableByDefault} core::int;
20+
b{core::int}.{core::int::isEven}{core::bool};
21+
block {
22+
final synthesized dynamic #0#0 = (3.14, {foo: "foo"});
23+
if(!((let final core::double #t2 = b = #0#0{(core::double, {foo: core::String})}.$1{core::double} in true) && true))
24+
throw new _in::ReachabilityError::•();
25+
} =>#0#0;
26+
return b;
27+
}
28+
static method test3() → dynamic {
29+
core::num b = 0;
30+
b as{ForNonNullableByDefault} core::int;
31+
b{core::int}.{core::int::isEven}{core::bool};
32+
block {
33+
final synthesized dynamic #0#0 = ({foo: 3.14});
34+
if(!(let final core::double #t3 = b = #0#0{({foo: core::double})}.foo{core::double} in true))
35+
throw new _in::ReachabilityError::•();
36+
} =>#0#0;
37+
return b;
38+
}
39+
40+
41+
Extra constant evaluation status:
42+
Evaluated: RecordLiteral @ org-dartlang-testcase:///issue51587.dart:9:10 -> RecordConstant(const (3.14))
43+
Evaluated: RecordLiteral @ org-dartlang-testcase:///issue51587.dart:17:17 -> RecordConstant(const (3.14, {foo: "foo"}))
44+
Evaluated: RecordLiteral @ org-dartlang-testcase:///issue51587.dart:25:14 -> RecordConstant(const ({foo: 3.14}))
45+
Extra constant evaluation: evaluated: 46, effectively constant: 3
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
test1() {}
2+
test2() {}
3+
test3() {}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
test1() {}
2+
test2() {}
3+
test3() {}

0 commit comments

Comments
 (0)