From 1af9f99a5f0240a9e9fc76e370a84cffbd949fc5 Mon Sep 17 00:00:00 2001 From: Kevin Millikin Date: Tue, 26 Mar 2019 13:45:52 +0000 Subject: [PATCH] [cfe] Compile loops in map literals Add support for compiling for and for-in loops in map literals. They are lowered to block expressions. Change-Id: I555401257ba028543310edbd0547166d7c6a26a2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97929 Reviewed-by: Dmitry Stefantsov --- .../lib/src/fasta/kernel/collections.dart | 18 ++-- .../src/fasta/kernel/inference_visitor.dart | 86 ++++++++++++++----- .../fasta/kernel/transform_collections.dart | 43 ++++++++++ .../testcases/control_flow_collection.dart | 11 ++- ...control_flow_collection.dart.legacy.expect | 40 ++++++--- ..._collection.dart.legacy.transformed.expect | 40 ++++++--- ...control_flow_collection.dart.strong.expect | 7 ++ ..._collection.dart.strong.transformed.expect | 7 ++ ...flow_collection.dart.type_promotion.expect | 3 + tests/co19_2/co19_2-kernel.status | 2 + tests/language_2/language_2_kernel.status | 5 -- 11 files changed, 199 insertions(+), 63 deletions(-) diff --git a/pkg/front_end/lib/src/fasta/kernel/collections.dart b/pkg/front_end/lib/src/fasta/kernel/collections.dart index 05cc74fe590e3..a20813f61f42f 100644 --- a/pkg/front_end/lib/src/fasta/kernel/collections.dart +++ b/pkg/front_end/lib/src/fasta/kernel/collections.dart @@ -200,25 +200,27 @@ class ForInElement extends Expression with ControlFlowElement { mixin ControlFlowMapEntry implements MapEntry { @override - Expression get key => throw UnsupportedError('SpreadMapEntry.key getter'); + Expression get key { + throw UnsupportedError('ControlFlowMapEntry.key getter'); + } @override void set key(Expression expr) { - throw UnsupportedError('SpreadMapEntry.key setter'); + throw UnsupportedError('ControlFlowMapEntry.key setter'); } @override - Expression get value => throw UnsupportedError('SpreadMapEntry.value getter'); + Expression get value { + throw UnsupportedError('ControlFlowMapEntry.value getter'); + } @override void set value(Expression expr) { - throw UnsupportedError('SpreadMapEntry.value setter'); + throw UnsupportedError('ControlFlowMapEntry.value setter'); } @override - accept(TreeVisitor v) { - throw UnsupportedError('SpreadMapEntry.accept'); - } + accept(TreeVisitor v) => v.defaultTreeNode(this); } /// A spread element in a map literal. @@ -230,8 +232,6 @@ class SpreadMapEntry extends TreeNode with ControlFlowMapEntry { expression?.parent = this; } - accept(TreeVisitor v) => v.defaultTreeNode(this); - @override visitChildren(Visitor v) { expression?.accept(v); diff --git a/pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart b/pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart index 4e50c703fc282..e0084e7eb5be7 100644 --- a/pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart +++ b/pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart @@ -693,7 +693,7 @@ class InferenceVisitor extends BodyVisitor1 { // TODO(kmillikin): Implement inference rules for if elements. inferrer.inferExpression(element.condition, inferrer.coreTypes.boolClass.rawType, typeChecksNeeded, - isVoidAllowed: false); // Should be void allowed + runtime check? + isVoidAllowed: false); inferElement(element.then, index, inferredTypeArgument, spreadTypes, inferenceNeeded, typeChecksNeeded); if (element.otherwise != null) { @@ -715,7 +715,7 @@ class InferenceVisitor extends BodyVisitor1 { element.condition, inferrer.coreTypes.boolClass.rawType, inferenceNeeded || typeChecksNeeded, - isVoidAllowed: false); // Should be void allowed + runtime check. + isVoidAllowed: false); } for (Expression expression in element.updates) { inferrer.inferExpression(expression, const UnknownType(), @@ -934,6 +934,7 @@ class InferenceVisitor extends BodyVisitor1 { } return spreadType; } else if (entry is IfMapEntry) { + // TODO(kmillikin): Implement inference rules for if map entries. inferrer.inferExpression(entry.condition, inferrer.coreTypes.boolClass.rawType, typeChecksNeeded, isVoidAllowed: false); @@ -966,25 +967,68 @@ class InferenceVisitor extends BodyVisitor1 { actualTypes.add(const DynamicType()); } return null; - } else if (entry is ForMapEntry || entry is ForInMapEntry) { - MapEntry replacement = new MapEntry( - new InvalidExpression('unhandled loop entry in map literal'), - new NullLiteral()) - ..fileOffset = entry.fileOffset; - if (nested) { - // VisitChildren doesn't work so replaceChild doesn't work either. - IfMapEntry parent = entry.parent; - replacement.parent = parent; - if (parent.then == entry) { - parent.then = replacement; - } else { - parent.otherwise = replacement; + } else if (entry is ForMapEntry) { + // TODO(kmillikin): Implement inference rules for for map entries. + for (VariableDeclaration declaration in entry.variables) { + if (declaration.initializer != null) { + inferrer.inferExpression(declaration.initializer, declaration.type, + inferenceNeeded || typeChecksNeeded, + isVoidAllowed: true); } - } else { - MapLiteral parent = entry.parent; - parent.entries[index] = replacement..parent = parent; - actualTypes.add(const BottomType()); - actualTypes.add(inferrer.coreTypes.nullClass.rawType); + } + if (entry.condition != null) { + inferrer.inferExpression( + entry.condition, + inferrer.coreTypes.boolClass.rawType, + inferenceNeeded || typeChecksNeeded, + isVoidAllowed: false); + } + for (Expression expression in entry.updates) { + inferrer.inferExpression(expression, const UnknownType(), + inferenceNeeded || typeChecksNeeded, + isVoidAllowed: true); + } + inferMapEntry( + entry.body, + index, + inferredKeyType, + inferredValueType, + spreadContext, + spreadTypes, + actualTypes, + inferenceNeeded, + typeChecksNeeded, + nested: true); + if (!nested) { + actualTypes.add(const DynamicType()); + actualTypes.add(const DynamicType()); + } + return null; + } else if (entry is ForInMapEntry) { + // TODO(kmillikin): Implement inference rules for for-in map entries. + inferForInIterable(entry.iterable, const UnknownType(), + inferenceNeeded || typeChecksNeeded, + isAsync: entry.isAsync); + if (entry.prologue != null) inferrer.inferStatement(entry.prologue); + if (entry.problem != null) { + inferrer.inferExpression(entry.problem, const UnknownType(), + inferenceNeeded || typeChecksNeeded, + isVoidAllowed: true); + } + inferMapEntry( + entry.body, + index, + inferredKeyType, + inferredValueType, + spreadContext, + spreadTypes, + actualTypes, + inferenceNeeded, + typeChecksNeeded, + nested: true); + if (!nested) { + actualTypes.add(const DynamicType()); + actualTypes.add(const DynamicType()); } return null; } else { @@ -1104,7 +1148,7 @@ class InferenceVisitor extends BodyVisitor1 { if (!isMap && isSet) { iterableSpreadOffset = entry.expression.fileOffset; } - } else if (entry is IfMapEntry) { + } else if (entry is ControlFlowMapEntry) { } else { mapEntryOffset = entry.fileOffset; } diff --git a/pkg/front_end/lib/src/fasta/kernel/transform_collections.dart b/pkg/front_end/lib/src/fasta/kernel/transform_collections.dart index 6e284131ee6f4..104edf8c51b88 100644 --- a/pkg/front_end/lib/src/fasta/kernel/transform_collections.dart +++ b/pkg/front_end/lib/src/fasta/kernel/transform_collections.dart @@ -49,6 +49,8 @@ import 'collections.dart' ControlFlowMapEntry, ForElement, ForInElement, + ForInMapEntry, + ForMapEntry, IfElement, IfMapEntry, SpreadElement, @@ -334,6 +336,10 @@ class CollectionTransformer extends Transformer { _translateSpreadEntry(entry, entryType, result, body); } else if (entry is IfMapEntry) { _translateIfEntry(entry, entryType, result, body); + } else if (entry is ForMapEntry) { + _translateForEntry(entry, entryType, result, body); + } else if (entry is ForInMapEntry) { + _translateForInEntry(entry, entryType, result, body); } else { _addNormalEntry(entry.accept(this), result, body); } @@ -368,6 +374,43 @@ class CollectionTransformer extends Transformer { entry.condition.accept(this), thenStatement, elseStatement)); } + void _translateForEntry(ForMapEntry entry, DartType entryType, + VariableDeclaration result, List body) { + List statements = []; + _translateEntry(entry.body, entryType, result, statements); + Statement loopBody = + statements.length == 1 ? statements.first : new Block(statements); + ForStatement loop = new ForStatement( + entry.variables, entry.condition?.accept(this), entry.updates, loopBody) + ..fileOffset = entry.fileOffset; + transformList(loop.variables, this, loop); + transformList(loop.updates, this, loop); + body.add(loop); + } + + void _translateForInEntry(ForInMapEntry entry, DartType entryType, + VariableDeclaration result, List body) { + List statements; + Statement prologue = entry.prologue; + if (prologue == null) { + statements = []; + } else { + prologue = prologue.accept(this); + statements = + prologue is Block ? prologue.statements : [prologue]; + } + _translateEntry(entry.body, entryType, result, statements); + Statement loopBody = + statements.length == 1 ? statements.first : new Block(statements); + if (entry.problem != null) { + body.add(new ExpressionStatement(entry.problem.accept(this))); + } + body.add(new ForInStatement( + entry.variable, entry.iterable.accept(this), loopBody, + isAsync: entry.isAsync) + ..fileOffset = entry.fileOffset); + } + void _translateSpreadEntry(SpreadMapEntry entry, DartType entryType, VariableDeclaration result, List body) { Expression value = entry.expression.accept(this); diff --git a/pkg/front_end/testcases/control_flow_collection.dart b/pkg/front_end/testcases/control_flow_collection.dart index 6c1ea1627aee4..987769f62071e 100644 --- a/pkg/front_end/testcases/control_flow_collection.dart +++ b/pkg/front_end/testcases/control_flow_collection.dart @@ -22,10 +22,13 @@ main() { for (int i = 11; i <= 14; ++i) i, }; final aMap = { - 1: 1, - if (oracle()) 2: 2, - if (oracle()) 3: 3 else -1: -1, - if (oracle()) if (oracle()) 4: 4, + 1: 1, + if (oracle()) 2: 2, + if (oracle()) 3: 3 else -1: -1, + if (oracle()) if (oracle()) 4: 4, + for (int i in [5, 6, 7]) i: i, + for (int i in [8, 9, 10]) if (oracle()) i: i, + for (int i = 11; i <= 14; ++i) i: i, }; print(aList); diff --git a/pkg/front_end/testcases/control_flow_collection.dart.legacy.expect b/pkg/front_end/testcases/control_flow_collection.dart.legacy.expect index d0d69e1bffeaa..099350f144b01 100644 --- a/pkg/front_end/testcases/control_flow_collection.dart.legacy.expect +++ b/pkg/front_end/testcases/control_flow_collection.dart.legacy.expect @@ -66,21 +66,37 @@ library; // for (int i = 11; i <= 14; ++i) i, // ^^^ // -// pkg/front_end/testcases/control_flow_collection.dart:26:5: Error: Unexpected token 'if'. -// if (oracle()) 2: 2, -// ^^ +// pkg/front_end/testcases/control_flow_collection.dart:26:7: Error: Unexpected token 'if'. +// if (oracle()) 2: 2, +// ^^ +// +// pkg/front_end/testcases/control_flow_collection.dart:27:7: Error: Unexpected token 'if'. +// if (oracle()) 3: 3 else -1: -1, +// ^^ +// +// pkg/front_end/testcases/control_flow_collection.dart:28:21: Error: Unexpected token 'if'. +// if (oracle()) if (oracle()) 4: 4, +// ^^ +// +// pkg/front_end/testcases/control_flow_collection.dart:28:7: Error: Unexpected token 'if'. +// if (oracle()) if (oracle()) 4: 4, +// ^^ +// +// pkg/front_end/testcases/control_flow_collection.dart:29:7: Error: Unexpected token 'for'. +// for (int i in [5, 6, 7]) i: i, +// ^^^ // -// pkg/front_end/testcases/control_flow_collection.dart:27:5: Error: Unexpected token 'if'. -// if (oracle()) 3: 3 else -1: -1, -// ^^ +// pkg/front_end/testcases/control_flow_collection.dart:30:38: Error: Unexpected token 'if'. +// for (int i in [8, 9, 10]) if (oracle()) i: i, +// ^^ // -// pkg/front_end/testcases/control_flow_collection.dart:28:19: Error: Unexpected token 'if'. -// if (oracle()) if (oracle()) 4: 4, -// ^^ +// pkg/front_end/testcases/control_flow_collection.dart:30:7: Error: Unexpected token 'for'. +// for (int i in [8, 9, 10]) if (oracle()) i: i, +// ^^^ // -// pkg/front_end/testcases/control_flow_collection.dart:28:5: Error: Unexpected token 'if'. -// if (oracle()) if (oracle()) 4: 4, -// ^^ +// pkg/front_end/testcases/control_flow_collection.dart:31:7: Error: Unexpected token 'for'. +// for (int i = 11; i <= 14; ++i) i: i, +// ^^^ // import self as self; import "dart:core" as core; diff --git a/pkg/front_end/testcases/control_flow_collection.dart.legacy.transformed.expect b/pkg/front_end/testcases/control_flow_collection.dart.legacy.transformed.expect index d0d69e1bffeaa..099350f144b01 100644 --- a/pkg/front_end/testcases/control_flow_collection.dart.legacy.transformed.expect +++ b/pkg/front_end/testcases/control_flow_collection.dart.legacy.transformed.expect @@ -66,21 +66,37 @@ library; // for (int i = 11; i <= 14; ++i) i, // ^^^ // -// pkg/front_end/testcases/control_flow_collection.dart:26:5: Error: Unexpected token 'if'. -// if (oracle()) 2: 2, -// ^^ +// pkg/front_end/testcases/control_flow_collection.dart:26:7: Error: Unexpected token 'if'. +// if (oracle()) 2: 2, +// ^^ +// +// pkg/front_end/testcases/control_flow_collection.dart:27:7: Error: Unexpected token 'if'. +// if (oracle()) 3: 3 else -1: -1, +// ^^ +// +// pkg/front_end/testcases/control_flow_collection.dart:28:21: Error: Unexpected token 'if'. +// if (oracle()) if (oracle()) 4: 4, +// ^^ +// +// pkg/front_end/testcases/control_flow_collection.dart:28:7: Error: Unexpected token 'if'. +// if (oracle()) if (oracle()) 4: 4, +// ^^ +// +// pkg/front_end/testcases/control_flow_collection.dart:29:7: Error: Unexpected token 'for'. +// for (int i in [5, 6, 7]) i: i, +// ^^^ // -// pkg/front_end/testcases/control_flow_collection.dart:27:5: Error: Unexpected token 'if'. -// if (oracle()) 3: 3 else -1: -1, -// ^^ +// pkg/front_end/testcases/control_flow_collection.dart:30:38: Error: Unexpected token 'if'. +// for (int i in [8, 9, 10]) if (oracle()) i: i, +// ^^ // -// pkg/front_end/testcases/control_flow_collection.dart:28:19: Error: Unexpected token 'if'. -// if (oracle()) if (oracle()) 4: 4, -// ^^ +// pkg/front_end/testcases/control_flow_collection.dart:30:7: Error: Unexpected token 'for'. +// for (int i in [8, 9, 10]) if (oracle()) i: i, +// ^^^ // -// pkg/front_end/testcases/control_flow_collection.dart:28:5: Error: Unexpected token 'if'. -// if (oracle()) if (oracle()) 4: 4, -// ^^ +// pkg/front_end/testcases/control_flow_collection.dart:31:7: Error: Unexpected token 'for'. +// for (int i = 11; i <= 14; ++i) i: i, +// ^^^ // import self as self; import "dart:core" as core; diff --git a/pkg/front_end/testcases/control_flow_collection.dart.strong.expect b/pkg/front_end/testcases/control_flow_collection.dart.strong.expect index a86c82b3c909d..9fed289b1166e 100644 --- a/pkg/front_end/testcases/control_flow_collection.dart.strong.expect +++ b/pkg/front_end/testcases/control_flow_collection.dart.strong.expect @@ -56,6 +56,13 @@ static method main() → dynamic { if(self::oracle()) if(self::oracle()) #t3.{core::Map::[]=}(4, 4); + for (core::int i in [5, 6, 7]) + #t3.{core::Map::[]=}(i, i); + for (core::int i in [8, 9, 10]) + if(self::oracle()) + #t3.{core::Map::[]=}(i, i); + for (core::int i = 11; i.{core::num::<=}(14); i = i.{core::num::+}(1)) + #t3.{core::Map::[]=}(i, i); } =>#t3; core::print(aList); core::print(aSet); diff --git a/pkg/front_end/testcases/control_flow_collection.dart.strong.transformed.expect b/pkg/front_end/testcases/control_flow_collection.dart.strong.transformed.expect index a86c82b3c909d..9fed289b1166e 100644 --- a/pkg/front_end/testcases/control_flow_collection.dart.strong.transformed.expect +++ b/pkg/front_end/testcases/control_flow_collection.dart.strong.transformed.expect @@ -56,6 +56,13 @@ static method main() → dynamic { if(self::oracle()) if(self::oracle()) #t3.{core::Map::[]=}(4, 4); + for (core::int i in [5, 6, 7]) + #t3.{core::Map::[]=}(i, i); + for (core::int i in [8, 9, 10]) + if(self::oracle()) + #t3.{core::Map::[]=}(i, i); + for (core::int i = 11; i.{core::num::<=}(14); i = i.{core::num::+}(1)) + #t3.{core::Map::[]=}(i, i); } =>#t3; core::print(aList); core::print(aSet); diff --git a/pkg/front_end/testcases/control_flow_collection.dart.type_promotion.expect b/pkg/front_end/testcases/control_flow_collection.dart.type_promotion.expect index ba820778fc837..54760970deba9 100644 --- a/pkg/front_end/testcases/control_flow_collection.dart.type_promotion.expect +++ b/pkg/front_end/testcases/control_flow_collection.dart.type_promotion.expect @@ -4,3 +4,6 @@ pkg/front_end/testcases/control_flow_collection.dart:13:33: Context: Write to i@ pkg/front_end/testcases/control_flow_collection.dart:22:33: Context: Write to i@364 for (int i = 11; i <= 14; ++i) i, ^^ +pkg/front_end/testcases/control_flow_collection.dart:31:33: Context: Write to i@364 + for (int i = 11; i <= 14; ++i) i: i, + ^^ diff --git a/tests/co19_2/co19_2-kernel.status b/tests/co19_2/co19_2-kernel.status index 97752491c9f04..260821a0d1fde 100644 --- a/tests/co19_2/co19_2-kernel.status +++ b/tests/co19_2/co19_2-kernel.status @@ -132,6 +132,7 @@ LanguageFeatures/Control-flow-collections/const_collections_A05_t02: CompileTime LanguageFeatures/Control-flow-collections/const_collections_A06_t02: CompileTimeError LanguageFeatures/Control-flow-collections/const_collections_A07_t01: CompileTimeError LanguageFeatures/Control-flow-collections/const_collections_A07_t02: CompileTimeError +LanguageFeatures/Control-flow-collections/dynamic_semantics_map_A03_t01: CompileTimeError LanguageFeatures/Control-flow-collections/dynamic_semantics_set_A03_t01: CompileTimeError LanguageFeatures/Control-flow-collections/static_errors_A01_t01/01: MissingCompileTimeError LanguageFeatures/Control-flow-collections/static_errors_A01_t01/02: MissingCompileTimeError @@ -181,6 +182,7 @@ LanguageFeatures/Control-flow-collections/static_semantics_A02_t02/21: MissingCo LanguageFeatures/Control-flow-collections/static_semantics_A02_t02/22: MissingCompileTimeError LanguageFeatures/Control-flow-collections/static_semantics_A02_t02/23: MissingCompileTimeError LanguageFeatures/Control-flow-collections/static_semantics_A02_t02/24: MissingCompileTimeError +LanguageFeatures/Control-flow-collections/syntax_A02_t01: CompileTimeError LanguageFeatures/Control-flow-collections/syntax_A03_t01/02: MissingCompileTimeError LanguageFeatures/Control-flow-collections/syntax_A03_t01/03: MissingCompileTimeError LanguageFeatures/Control-flow-collections/type_inference_A06_t01: CompileTimeError diff --git a/tests/language_2/language_2_kernel.status b/tests/language_2/language_2_kernel.status index d12394f0797d1..fd81b5e7ee814 100644 --- a/tests/language_2/language_2_kernel.status +++ b/tests/language_2/language_2_kernel.status @@ -202,7 +202,6 @@ constructor5_test: CompileTimeError # Verification error constructor6_test: CompileTimeError # Verification error control_flow_collections/await_for_inference_test: CompileTimeError control_flow_collections/await_for_test: CompileTimeError -control_flow_collections/await_for_type_error_test/02: MissingCompileTimeError control_flow_collections/await_for_type_error_test/04: MissingCompileTimeError control_flow_collections/await_for_type_error_test/05: MissingCompileTimeError control_flow_collections/await_for_type_error_test/06: MissingCompileTimeError @@ -214,10 +213,7 @@ control_flow_collections/await_for_type_error_test/13: MissingCompileTimeError control_flow_collections/for_await_test: CompileTimeError control_flow_collections/for_await_test: Crash control_flow_collections/for_const_test/00: MissingCompileTimeError -control_flow_collections/for_const_test/01: MissingCompileTimeError control_flow_collections/for_const_test/03: MissingCompileTimeError -control_flow_collections/for_const_test/04: MissingCompileTimeError -control_flow_collections/for_const_test/07: MissingCompileTimeError control_flow_collections/for_inference_test: CompileTimeError control_flow_collections/for_test: CompileTimeError control_flow_collections/if_await_test: Crash @@ -265,7 +261,6 @@ control_flow_collections/type_error_test/26: MissingCompileTimeError control_flow_collections/type_error_test/27: MissingCompileTimeError control_flow_collections/type_error_test/28: MissingCompileTimeError control_flow_collections/type_error_test/29: MissingCompileTimeError -control_flow_collections/type_error_test/31: MissingCompileTimeError control_flow_collections/type_error_test/33: MissingCompileTimeError control_flow_collections/type_error_test/34: MissingCompileTimeError control_flow_collections/type_error_test/35: MissingCompileTimeError