Add ScopeInfo class for variable information that doesn't actually involve closures.

pkg/compiler/lib/src/closure.dart

 // Copyright (c) 2012, 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.
 
 import 'common/names.dart' show Identifiers;
 import 'common/resolution.dart' show ParsingContext, Resolution;
 import 'common/tasks.dart' show CompilerTask, Measurer;
 import 'common.dart';
 import 'compiler.dart' show Compiler;
 import 'constants/expressions.dart';
 import 'elements/elements.dart';
 import 'elements/entities.dart';
 import 'elements/entity_utils.dart' as utils;
 import 'elements/modelx.dart'
     show BaseFunctionElementX, ClassElementX, ElementX;
 import 'elements/resolution_types.dart';
 import 'elements/types.dart';
 import 'elements/visitor.dart' show ElementVisitor;
 import 'js_backend/js_backend.dart' show JavaScriptBackend;
 import 'resolution/tree_elements.dart' show TreeElements;
 import 'package:front_end/src/fasta/scanner.dart' show Token;
 import 'tree/tree.dart';
 import 'util/util.dart';
 import 'world.dart' show ClosedWorldRefiner;
+import 'package:collection/collection.dart' show UnmodifiableSetView;
 
 // TODO(johnniwinther,efortuna): Split [ClosureConversionTask] from
 // [ClosureDataLookup].
 abstract class ClosureConversionTask<T> extends CompilerTask
     implements ClosureDataLookup<T> {
   ClosureConversionTask(Measurer measurer) : super(measurer);
 
   //void analyzeClosures();
   void convertClosures(Iterable<MemberEntity> processedEntities,
       ClosedWorldRefiner closedWorldRefiner);
 }
 
 /// Class that provides information for how closures are rewritten/represented
 /// to preserve Dart semantics when compiled to JavaScript. Given a particular
 /// node to look up, it returns a information about the internal representation
 /// of how closure conversion is implemented. T is an ir.Node or Node.
 abstract class ClosureDataLookup<T> {
   /// Look up information about the variables that have been mutated and are
   /// used inside the scope of [node].
   // TODO(johnniwinther): Split this up into two functions, one for members and
   // one for local functions.
+  ScopeInfo getScopeInfo(covariant Entity member);
+
+  /// This returns the same information as ScopeInfo, but can be called in
+  /// situations when you are sure you are dealing with a closure specifically.
   ClosureRepresentationInfo getClosureRepresentationInfo(
       covariant Entity member);
 
   /// Look up information about a loop, in case any variables it declares need
   /// to be boxed/snapshotted.
   LoopClosureRepresentationInfo getClosureRepresentationInfoForLoop(T loopNode);
 
   /// Accessor to the information about closures that the SSA builder will use.
   ClosureAnalysisInfo getClosureAnalysisInfo(T node);
 }
 
+/// Class that represents one level of scoping information, whether this scope
+/// is a closure or not. This is specifically used to store information
+/// about the usage of variables in try or sync blocks, because they need to be
+/// boxed.
+///
+/// Variables that are used in a try must be treated as boxed because the
+/// control flow can be non-linear. Also parameters to a `sync*` generator must
+/// be boxed, because of the way we rewrite sync* functions. See also comments
+/// in [ClosureClassMap.useLocal].
+class ScopeInfo {
+  const ScopeInfo();
+
+  /// Set of [variable]s referenced in this scope that are used inside a
+  /// `try` block or a `sync*` generator (this is important to know because
+  /// boxing/redirection needs to happen for those local variables).
+  ///
+  /// Variables that are used in a try must be treated as boxed because the
+  /// control flow can be non-linear.
+  ///
+  /// Also parameters to a `sync*` generator must be boxed, because of the way
+  /// we rewrite sync* functions. See also comments in
+  /// [ClosureClassMap.useLocal].
+  // TODO(johnniwinther): Add variables to this only if the variable is mutated.
+  final Set<Local> variablesUsedInTryOrSync = const UnmodifiableSetView.empty();

[Siggi Cherem (dart-lang), 2017/06/23 19:11:56]

I lean towards simply using `new Set<Local>()` here instead and drop the
dependency on package:collection.

[Emily Fortuna, 2017/06/23 21:20:06]

switched to old API!

+
+  /// Convenience reference pointer to the element representing `this`.
+  /// If this scope is not in an instance member, it will be null.
+  Local get thisLocal => null;
+}
+
 /// Class that provides a black-box interface to information gleaned from
 /// analyzing a closure's characteristics, most commonly used to influence how
 /// code should be generated in SSA builder stage.
 class ClosureAnalysisInfo {
   const ClosureAnalysisInfo();
 
   /// If true, this closure accesses a variable that was defined in an outside
   /// scope and this variable gets modified at some point (sometimes we say that
   /// variable has been "captured"). In this situation, access to this variable
   /// is controlled via a wrapper (box) so that updates to this variable
@@ skipping 64 matching lines @@
 ///     var foo = new FooClosure(1);
 ///     foo.call(2);
 ///
 /// if `y` is modified elsewhere within its scope, accesses to y anywhere in the
 /// code will be controlled via a box object.
 ///
 /// Because in these examples `y` was declared in some other, outer scope, but
 /// used in the inner scope of this closure, we say `y` is a "captured"
 /// variable.
 /// TODO(efortuna): Make interface simpler in subsequent refactorings.
-class ClosureRepresentationInfo {
+class ClosureRepresentationInfo extends ScopeInfo {
   const ClosureRepresentationInfo();
 
   /// The original local function before any translation.
   ///
   /// Will be null for methods.
   Local get closureEntity => null;
 
   /// The entity for the class used to represent the rewritten closure in the
   /// emitted JavaScript.
   ///
   /// Closures are rewritten in the form of classes that have fields to control
   /// the redirection and editing of captured variables.
   ClassEntity get closureClassEntity => null;
 
   /// The function that implements the [local] function as a `call` method on
   /// the closure class.
   FunctionEntity get callMethod => null;
 
   /// As shown in the example in the comments at the top of this class, we
   /// create fields in the closure class for each captured variable. This is an
   /// accessor to that set of fields.
   List<Local> get createdFieldEntities => const <Local>[];
 
-  /// Convenience reference pointer to the element representing `this`.
-  /// It is only set for instance-members.
-  Local get thisLocal => null;
-
   /// Convenience pointer to the field entity representation in the closure
   /// class of the element representing `this`.
   FieldEntity get thisFieldEntity => null;
 
-  /// Returns true if this [variable] is used inside a `try` block or a `sync*`
-  /// generator (this is important to know because boxing/redirection needs to
-  /// happen for those local variables).
-  ///
-  /// Variables that are used in a try must be treated as boxed because the
-  /// control flow can be non-linear.
-  ///
-  /// Also parameters to a `sync*` generator must be boxed, because of the way
-  /// we rewrite sync* functions. See also comments in
-  /// [ClosureClassMap.useLocal].
-  bool variableIsUsedInTryOrSync(Local variable) => false;

[Siggi Cherem (dart-lang), 2017/06/23 19:11:56]

I think it's fine either way, but what I liked about having this bool-API is
that the `Set` implementation was hidden from consumers of this info.

It makes sense to expose the set so you can directly add to it in the
kernel-specific implementation though. So I can see that this API provides less
value now that the set is exposed.

[Emily Fortuna, 2017/06/23 21:20:06]

Moved back to the API it was!

-
   /// Loop through every variable that has been captured in this closure. This
   /// consists of all the free variables (variables captured *just* in this
   /// closure) and all variables captured in nested scopes that we may be
   /// capturing as well. These nested scopes hold "boxes" to hold the executable
   /// context for that scope.
   void forEachCapturedVariable(f(Local from, FieldEntity to)) {}
 
   /// Loop through each variable that has been boxed in this closure class. Only
   /// captured variables that are mutated need to be "boxed" (which basically
   /// puts a thin layer between updates and reads to this variable to ensure
@@ skipping 31 matching lines @@
   void convertClosures(Iterable<MemberEntity> processedEntities,
       ClosedWorldRefiner closedWorldRefiner) {
     createClosureClasses(closedWorldRefiner);
   }
 
   ClosureAnalysisInfo getClosureAnalysisInfo(Node node) {
     var value = _closureInfoMap[node];
     return value == null ? const ClosureAnalysisInfo() : value;
   }
 
+  ScopeInfo getScopeInfo(Element member) {
+    return getClosureToClassMapping(member);
+  }
+
   ClosureRepresentationInfo getClosureRepresentationInfo(Element member) {
     return getClosureToClassMapping(member);
   }
 
   LoopClosureRepresentationInfo getClosureRepresentationInfoForLoop(
       Node loopNode) {
     var value = _closureInfoMap[loopNode];
     return value == null ? const LoopClosureRepresentationInfo() : value;
   }
 
@@ skipping 437 matching lines @@
   /// Maps free locals, arguments, function elements, and box locals to
   /// their locations.
   final Map<Local, FieldEntity> freeVariableMap = new Map<Local, FieldEntity>();
 
   /// Maps [Loop] and [FunctionExpression] nodes to their [ClosureScope] which
   /// contains their box and the captured variables that are stored in the box.
   /// This map will be empty if the method/closure of this [ClosureData] does
   /// not contain any nested closure.
   final Map<Node, ClosureScope> capturingScopes = new Map<Node, ClosureScope>();
 
-  /// Variables that are used in a try must be treated as boxed because the
-  /// control flow can be non-linear.
-  ///
-  /// Also parameters to a `sync*` generator must be boxed, because of the way
-  /// we rewrite sync* functions. See also comments in [useLocal].
-  // TODO(johnniwinther): Add variables to this only if the variable is mutated.
-  final Set<Local> variablesUsedInTryOrGenerator = new Set<Local>();
+  final Set<Local> variablesUsedInTryOrSync = new Set<Local>();

[Siggi Cherem (dart-lang), 2017/06/23 19:11:56]

alternatively, we could change "implements ClosureRepresentationInfo" to
"extends ...", and use the field from the supersuperclass

[Emily Fortuna, 2017/06/23 21:20:06]

Acknowledged.

 
   ClosureClassMap(this.closureEntity, this.closureClassEntity, this.callMethod,
       this.thisLocal);
 
   List<Local> get createdFieldEntities {
     List<Local> fields = <Local>[];
     if (closureClassEntity == null) return const <Local>[];
     closureClassEntity.closureFields.forEach((field) {
       fields.add(field.local);
     });
@@ skipping 10 matching lines @@
   bool isFreeVariable(Local element) {
     return freeVariableMap.containsKey(element);
   }
 
   void forEachFreeVariable(f(Local variable, FieldEntity field)) {
     freeVariableMap.forEach(f);
   }
 
   FieldEntity get thisFieldEntity => freeVariableMap[thisLocal];
 
-  bool variableIsUsedInTryOrSync(Local variable) =>
-      variablesUsedInTryOrGenerator.contains(variable);
-
   Local getLocalVariableForClosureField(ClosureFieldElement field) {
     return field.local;
   }
 
   bool get isClosure => closureEntity != null;
 
   bool capturingScopesBox(Local variable) {
     return capturingScopes.values.any((scope) {
       return scope.boxedLoopVariables.contains(variable);
     });
@@ skipping 219 matching lines @@
 
     if (insideClosure && !inCurrentContext(variable)) {
       closureData.addFreeVariable(variable);
     } else if (inTryStatement) {
       // Don't mark the this-element or a self-reference. This would complicate
       // things in the builder.
       // Note that nested (named) functions are immutable.
       if (variable != closureData.thisLocal &&
           variable != closureData.closureEntity &&
           variable is! TypeVariableLocal) {
-        closureData.variablesUsedInTryOrGenerator.add(variable);
+        closureData.variablesUsedInTryOrSync.add(variable);
       }
     } else if (variable is LocalParameterElement &&
         variable.functionDeclaration.asyncMarker == AsyncMarker.SYNC_STAR) {
       // Parameters in a sync* function are shared between each Iterator created
       // by the Iterable returned by the function, therefore they must be boxed.
-      closureData.variablesUsedInTryOrGenerator.add(variable);
+      closureData.variablesUsedInTryOrSync.add(variable);
     }
   }
 
   void useTypeVariableAsLocal(ResolutionTypeVariableType typeVariable) {
     useLocal(new TypeVariableLocal(
         typeVariable, outermostElement, outermostElement.memberContext));
   }
 
   void declareLocal(LocalVariableElement element) {
     scopeVariables.add(element);
@@ skipping 479 matching lines @@
 ///
 /// Move the below classes to a JS model eventually.
 ///
 abstract class JSEntity implements MemberEntity {
   Local get declaredEntity;
 }
 
 abstract class PrivatelyNamedJSEntity implements JSEntity {
   Entity get rootOfScope;
 }