Skip to content

Commit

Permalink
[cfe] Avoid repearing type inference on pattern for-in loops
Browse files Browse the repository at this point in the history
Previously the type inference was repeated in order to obtain the
static type of the iterable expression. That type is computed in the
shared analysis, and this CL passes the computed type to the clients
of the analysis routine in the result object.

In addition to being redundant, the repeated inference pass caused
crashes.

Closes #53092

Change-Id: I63d3d82be046884fa16d904a43e3bb6e01cf6f7e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/321141
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
  • Loading branch information
chloestefantsova authored and Commit Queue committed Aug 17, 2023
1 parent 8c5f895 commit 47420c0
Show file tree
Hide file tree
Showing 13 changed files with 36 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -320,11 +320,15 @@ class PatternForInResult<Type extends Object, Error> {
/// The static type of the elements of the for in expression.
final Type elementType;

/// The static type of the collection of elements of the for in expression.
final Type expressionType;

/// Error for when the expression is not an iterable.
final Error? patternForInExpressionIsNotIterableError;

PatternForInResult(
{required this.elementType,
required this.expressionType,
required this.patternForInExpressionIsNotIterableError});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1425,6 +1425,7 @@ mixin TypeAnalyzer<

return new PatternForInResult(
elementType: elementType,
expressionType: expressionType,
patternForInExpressionIsNotIterableError:
patternForInExpressionIsNotIterableError);
}
Expand Down
24 changes: 16 additions & 8 deletions pkg/front_end/lib/src/fasta/type_inference/inference_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1636,20 +1636,28 @@ class InferenceVisitorImpl extends InferenceVisitorBase
ForInVariable forInVariable =
new PatternVariableDeclarationForInVariable(patternVariableDeclaration);

DartType elementType = forInVariable.computeElementType(this);
ExpressionInferenceResult iterableResult =
inferForInIterable(iterable, elementType, isAsync: isAsync);
DartType inferredType = iterableResult.inferredType;
variable.type = inferredType;
variable.type = result.elementType;
iterable = ensureAssignable(
wrapType(
const DynamicType(),
isAsync ? coreTypes.streamClass : coreTypes.iterableClass,
libraryBuilder.nonNullable),
result.expressionType,
iterable,
errorTemplate: templateForInLoopTypeNotIterable,
nullabilityErrorTemplate: templateForInLoopTypeNotIterableNullability,
nullabilityPartErrorTemplate:
templateForInLoopTypeNotIterablePartNullability);
// This is matched by the call to [forEach_end] in
// [inferElement], [inferMapEntry] or [inferForInStatement].
flowAnalysis.forEach_bodyBegin(node);
syntheticAssignment = forInVariable.inferAssignment(this, inferredType);
syntheticAssignment =
forInVariable.inferAssignment(this, result.elementType);
if (syntheticAssignment is VariableSet) {
flowAnalysis.write(node, variable, inferredType, null);
flowAnalysis.write(node, variable, result.elementType, null);
}

return new ForInResult(variable, iterableResult.expression,
return new ForInResult(variable, /*iterableResult.expression*/ iterable,
syntheticAssignment, patternVariableDeclaration);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import self as self;
import "dart:core" as core;

static method test() → dynamic {
for (final dynamic #t1 in invalid-expression "pkg/front_end/testcases/patterns/for_in_inference_error.dart:6:23: Error: The type 'int' used in the 'for' loop must implement 'Iterable<dynamic>'.
for (final invalid-type #t1 in invalid-expression "pkg/front_end/testcases/patterns/for_in_inference_error.dart:6:23: Error: The type 'int' used in the 'for' loop must implement 'Iterable<dynamic>'.
- 'Iterable' is from 'dart:core'.
for (var [int x] in 0) {} // Error.
^" in 0 as{TypeError} core::Iterable<dynamic>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ static method test() → dynamic {
for (var [int x] in 0) {} // Error.
^" in 0 as{TypeError} core::Iterable<dynamic>.{core::Iterable::iterator}{core::Iterator<Never>};
for (; :sync-for-iterator.{core::Iterator::moveNext}(){() → core::bool}; ) {
final dynamic #t1 = :sync-for-iterator.{core::Iterator::current}{Never};
final invalid-type #t1 = :sync-for-iterator.{core::Iterator::current}{Never};
{
hoisted core::int x;
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import self as self;
import "dart:core" as core;

static method test() → dynamic {
for (final dynamic #t1 in invalid-expression "pkg/front_end/testcases/patterns/for_in_inference_error.dart:6:23: Error: The type 'int' used in the 'for' loop must implement 'Iterable<dynamic>'.
for (final invalid-type #t1 in invalid-expression "pkg/front_end/testcases/patterns/for_in_inference_error.dart:6:23: Error: The type 'int' used in the 'for' loop must implement 'Iterable<dynamic>'.
- 'Iterable' is from 'dart:core'.
for (var [int x] in 0) {} // Error.
^" in 0 as{TypeError} core::Iterable<dynamic>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import self as self;
import "dart:core" as core;

static method test() → dynamic {
for (final dynamic #t1 in invalid-expression "pkg/front_end/testcases/patterns/for_in_inference_error.dart:6:23: Error: The type 'int' used in the 'for' loop must implement 'Iterable<dynamic>'.
for (final invalid-type #t1 in invalid-expression "pkg/front_end/testcases/patterns/for_in_inference_error.dart:6:23: Error: The type 'int' used in the 'for' loop must implement 'Iterable<dynamic>'.
- 'Iterable' is from 'dart:core'.
for (var [int x] in 0) {} // Error.
^" in 0 as{TypeError} core::Iterable<dynamic>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ static method test() → dynamic {
for (var [int x] in 0) {} // Error.
^" in 0 as{TypeError} core::Iterable<dynamic>.{core::Iterable::iterator}{core::Iterator<Never>};
for (; :sync-for-iterator.{core::Iterator::moveNext}(){() → core::bool}; ) {
final dynamic #t1 = :sync-for-iterator.{core::Iterator::current}{Never};
final invalid-type #t1 = :sync-for-iterator.{core::Iterator::current}{Never};
{
hoisted core::int x;
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ library;
// ^^^^^
//
import self as self;
import "dart:async" as asy;
import "dart:core" as core;
import "dart:async" as asy;

static method test() → dynamic {
await for (final dynamic #t1 in asy::Stream::fromIterable<dynamic>(<core::List<core::int>>[<core::int>[1], <core::int>[2], <core::int>[3]])) {
await for (final core::List<core::int> #t1 in asy::Stream::fromIterable<core::List<core::int>>(<core::List<core::int>>[<core::int>[1], <core::int>[2], <core::int>[3]])) {
hoisted core::int v1;
{
final synthesized core::List<core::int> #0#0 = #t1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ library;
// ^^^^^
//
import self as self;
import "dart:async" as asy;
import "dart:core" as core;
import "dart:async" as asy;

static method test() → dynamic {
await for (final dynamic #t1 in asy::Stream::fromIterable<dynamic>(core::_GrowableList::_literal3<core::List<core::int>>(core::_GrowableList::_literal1<core::int>(1), core::_GrowableList::_literal1<core::int>(2), core::_GrowableList::_literal1<core::int>(3)))) {
await for (final core::List<core::int> #t1 in asy::Stream::fromIterable<core::List<core::int>>(core::_GrowableList::_literal3<core::List<core::int>>(core::_GrowableList::_literal1<core::int>(1), core::_GrowableList::_literal1<core::int>(2), core::_GrowableList::_literal1<core::int>(3)))) {
hoisted core::int v1;
{
final synthesized core::List<core::int> #0#0 = #t1;
Expand Down
4 changes: 2 additions & 2 deletions pkg/front_end/testcases/patterns/issue52228.dart.weak.expect
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ library;
// ^^^^^
//
import self as self;
import "dart:async" as asy;
import "dart:core" as core;
import "dart:async" as asy;

static method test() → dynamic {
await for (final dynamic #t1 in asy::Stream::fromIterable<dynamic>(<core::List<core::int>>[<core::int>[1], <core::int>[2], <core::int>[3]])) {
await for (final core::List<core::int> #t1 in asy::Stream::fromIterable<core::List<core::int>>(<core::List<core::int>>[<core::int>[1], <core::int>[2], <core::int>[3]])) {
hoisted core::int v1;
{
final synthesized core::List<core::int> #0#0 = #t1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ library;
// ^^^^^
//
import self as self;
import "dart:async" as asy;
import "dart:core" as core;
import "dart:async" as asy;

static method test() → dynamic {
await for (final dynamic #t1 in asy::Stream::fromIterable<dynamic>(<core::List<core::int>>[<core::int>[1], <core::int>[2], <core::int>[3]])) {
await for (final core::List<core::int> #t1 in asy::Stream::fromIterable<core::List<core::int>>(<core::List<core::int>>[<core::int>[1], <core::int>[2], <core::int>[3]])) {
hoisted core::int v1;
{
final synthesized core::List<core::int> #0#0 = #t1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ library;
// ^^^^^
//
import self as self;
import "dart:async" as asy;
import "dart:core" as core;
import "dart:async" as asy;

static method test() → dynamic {
await for (final dynamic #t1 in asy::Stream::fromIterable<dynamic>(core::_GrowableList::_literal3<core::List<core::int>>(core::_GrowableList::_literal1<core::int>(1), core::_GrowableList::_literal1<core::int>(2), core::_GrowableList::_literal1<core::int>(3)))) {
await for (final core::List<core::int> #t1 in asy::Stream::fromIterable<core::List<core::int>>(core::_GrowableList::_literal3<core::List<core::int>>(core::_GrowableList::_literal1<core::int>(1), core::_GrowableList::_literal1<core::int>(2), core::_GrowableList::_literal1<core::int>(3)))) {
hoisted core::int v1;
{
final synthesized core::List<core::int> #0#0 = #t1;
Expand Down

0 comments on commit 47420c0

Please sign in to comment.