[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 <dmitryas@google.com>
flutter · Mar 26, 2019 · 1af9f99 · 1af9f99
1 parent 4c48062
commit 1af9f99
Show file tree

Hide file tree

Showing 11 changed files with 199 additions and 63 deletions.
diff --git 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<Object> v) {
-    throw UnsupportedError('SpreadMapEntry.accept');
-  }
+  accept(TreeVisitor<Object> 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<Object> v) => v.defaultTreeNode(this);
-
   @override
   visitChildren(Visitor<Object> 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
@@ -693,7 +693,7 @@ class InferenceVisitor extends BodyVisitor1<void, DartType> {
       // 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<void, DartType> {
             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<void, DartType> {
       }
       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<void, DartType> {
         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<void, DartType> {
           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
@@ -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<Statement> body) {
+    List<Statement> statements = <Statement>[];
+    _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<Statement> body) {
+    List<Statement> statements;
+    Statement prologue = entry.prologue;
+    if (prologue == null) {
+      statements = <Statement>[];
+    } else {
+      prologue = prologue.accept(this);
+      statements =
+          prologue is Block ? prologue.statements : <Statement>[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<Statement> 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
@@ -22,10 +22,13 @@ main() {
       for (int i = 11; i <= 14; ++i) i,
   };
   final aMap = <int, int>{
-    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 <int>[5, 6, 7]) i: i,
+      for (int i in <int>[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
@@ -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 <int>[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 <int>[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 <int>[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
@@ -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 <int>[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 <int>[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 <int>[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
@@ -56,6 +56,13 @@ static method main() → dynamic {
     if(self::oracle())
       if(self::oracle())
         #t3.{core::Map::[]=}(4, 4);
+    for (core::int i in <core::int>[5, 6, 7])
+      #t3.{core::Map::[]=}(i, i);
+    for (core::int i in <core::int>[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
@@ -56,6 +56,13 @@ static method main() → dynamic {
     if(self::oracle())
       if(self::oracle())
         #t3.{core::Map::[]=}(4, 4);
+    for (core::int i in <core::int>[5, 6, 7])
+      #t3.{core::Map::[]=}(i, i);
+    for (core::int i in <core::int>[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
@@ -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
@@ -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