Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(518)

Unified Diff: pkg/compiler/lib/src/js_model/closure.dart

Issue 2986583002: Reapply "Add nonboxed closure class fields." (Closed)
Patch Set: Created 3 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | pkg/compiler/lib/src/js_model/closure_visitors.dart » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: pkg/compiler/lib/src/js_model/closure.dart
diff --git a/pkg/compiler/lib/src/js_model/closure.dart b/pkg/compiler/lib/src/js_model/closure.dart
index 58fa5c777e5b33559dbec10642bceec1a056b490..8edd29973157bf0cedd8300ba36f4fbd6e19b39c 100644
--- a/pkg/compiler/lib/src/js_model/closure.dart
+++ b/pkg/compiler/lib/src/js_model/closure.dart
@@ -6,7 +6,10 @@ import 'package:kernel/ast.dart' as ir;
import '../closure.dart';
import '../common/tasks.dart';
+import '../elements/elements.dart';
import '../elements/entities.dart';
+import '../elements/entity_utils.dart' as utils;
+import '../elements/names.dart' show Name;
import '../kernel/element_map.dart';
import '../world.dart';
import 'elements.dart';
@@ -34,8 +37,7 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
/// Map of the scoping information that corresponds to a particular entity.
Map<Entity, ScopeInfo> _scopeMap = <Entity, ScopeInfo>{};
- Map<ir.Node, CapturedScope> _scopesCapturedInClosureMap =
- <ir.Node, CapturedScope>{};
+ Map<ir.Node, CapturedScope> _capturedScopesMap = <ir.Node, CapturedScope>{};
Map<Entity, ClosureRepresentationInfo> _closureRepresentationMap =
<Entity, ClosureRepresentationInfo>{};
@@ -60,10 +62,7 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
ClosedWorldRefiner closedWorldRefiner) {
var closuresToGenerate = <ir.TreeNode, ScopeInfo>{};
processedEntities.forEach((MemberEntity kEntity) {
- MemberEntity entity = kEntity;
- if (_kToJElementMap != null) {
- entity = _kToJElementMap.toBackendMember(kEntity);
- }
+ MemberEntity entity = _kToJElementMap.toBackendMember(kEntity);
if (entity.isAbstract) return;
if (entity.isField && !entity.isInstanceMember) {
ir.Field field = _elementMap.getMemberNode(entity);
@@ -87,9 +86,9 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
ClosedWorldRefiner closedWorldRefiner) {
if (_scopeMap.keys.contains(entity)) return;
ir.Node node = _elementMap.getMemberNode(entity);
- if (_scopesCapturedInClosureMap.keys.contains(node)) return;
+ if (_capturedScopesMap.keys.contains(node)) return;
CapturedScopeBuilder translator = new CapturedScopeBuilder(
- _scopesCapturedInClosureMap,
+ _capturedScopesMap,
_scopeMap,
entity,
closuresToGenerate,
@@ -107,14 +106,36 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
/// Given what variables are captured at each point, construct closure classes
/// with fields containing the captured variables to replicate the Dart
- /// closure semantics in JS.
+ /// closure semantics in JS. If this closure captures any variables (meaning
+ /// the closure accesses a variable that gets accessed at some point), then
+ /// boxForCapturedVariables stores the local context for those variables.
+ /// If no variables are captured, this parameter is null.
void _produceSyntheticElements(
ir.TreeNode /* ir.Field | ir.FunctionNode */ node,
ScopeInfo info,
ClosedWorldRefiner closedWorldRefiner) {
Entity entity;
- KernelClosureClass closureClass =
- new KernelClosureClass.fromScopeInfo(info);
+ ir.Library library;
+ if (node is ir.Member) {
+ entity = _elementMap.getMember(node);
+ library = node.enclosingLibrary;
+ } else {
+ entity = _elementMap.getLocalFunction(node);
+ // TODO(efortuna): Consider the less roundabout way of getting this value
+ // which is just storing the "enclosingLibrary" value of the original call
+ // to CapturedScopeBuilder.
+ ir.TreeNode temp = node;
+ while (temp != null && temp is! ir.Library) {
+ temp = temp.parent;
+ }
+ assert(temp is ir.Library);
+ library = temp;
+ }
+ assert(entity != null);
+
+ String name = _computeClosureName(node);
+ KernelClosureClass closureClass = new KernelClosureClass.fromScopeInfo(
+ name, _elementMap.getLibrary(library), info);
if (node is ir.FunctionNode) {
// We want the original declaration where that function is used to point
// to the correct closure class.
@@ -123,13 +144,6 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
_closureRepresentationMap[closureClass.callMethod] = closureClass;
}
- if (node is ir.Member) {
- entity = _elementMap.getMember(node);
- } else {
- entity = _elementMap.getLocalFunction(node);
- }
- assert(entity != null);
-
_closureRepresentationMap[entity] = closureClass;
// Register that a new class has been created.
@@ -137,6 +151,48 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
closureClass, node is ir.Member && node.isInstanceMember);
}
+ // Returns a non-unique name for the given closure element.
+ String _computeClosureName(ir.TreeNode treeNode) {
+ var parts = <String>[];
+ if (treeNode is ir.Field && treeNode.name.name != "") {
+ parts.add(treeNode.name.name);
+ } else {
+ parts.add('closure');
+ }
+ ir.TreeNode node = treeNode.parent;
+ while (node != null &&
+ (node is ir.Constructor ||
+ node is ir.Class ||
+ node is ir.FunctionNode ||
+ node is ir.Procedure)) {
+ // TODO(johnniwinther): Simplify computed names.
+ if (node is ir.Constructor ||
+ node.parent is ir.Constructor ||
+ (node is ir.Procedure && node.kind == ir.ProcedureKind.Factory)) {
+ FunctionEntity entity;
+ if (node.parent is ir.Constructor) {
+ entity = _elementMap.getConstructorBody(node);
+ } else {
+ entity = _elementMap.getMember(node);
+ }
+ parts.add(utils.reconstructConstructorName(entity));
+ } else {
+ String surroundingName = '';
+ if (node is ir.Class) {
+ surroundingName = Elements.operatorNameToIdentifier(node.name);
+ } else if (node is ir.Procedure) {
+ surroundingName = Elements.operatorNameToIdentifier(node.name.name);
+ }
+ parts.add(surroundingName);
+ }
+ // A generative constructors's parent is the class; the class name is
+ // already part of the generative constructor's name.
+ if (node is ir.Constructor) break;
+ node = node.parent;
+ }
+ return parts.reversed.join('_');
+ }
+
@override
ScopeInfo getScopeInfo(Entity entity) {
// TODO(johnniwinther): Remove this check when constructor bodies a created
@@ -150,18 +206,18 @@ class KernelClosureConversionTask extends ClosureConversionTask<ir.Node> {
return _scopeMap[entity] ?? getClosureRepresentationInfo(entity);
}
- // TODO(efortuna): Eventually scopesCapturedInClosureMap[node] should always
+ // TODO(efortuna): Eventually capturedScopesMap[node] should always
// be non-null, and we should just test that with an assert.
@override
CapturedScope getCapturedScope(MemberEntity entity) =>
- _scopesCapturedInClosureMap[_elementMap.getMemberNode(entity)] ??
+ _capturedScopesMap[_elementMap.getMemberNode(entity)] ??
const CapturedScope();
@override
- // TODO(efortuna): Eventually scopesCapturedInClosureMap[node] should always
+ // TODO(efortuna): Eventually capturedScopesMap[node] should always
// be non-null, and we should just test that with an assert.
CapturedLoopScope getCapturedLoopScope(ir.Node loopNode) =>
- _scopesCapturedInClosureMap[loopNode] ?? const CapturedLoopScope();
+ _capturedScopesMap[loopNode] ?? const CapturedLoopScope();
@override
// TODO(efortuna): Eventually closureRepresentationMap[node] should always be
@@ -181,16 +237,24 @@ class KernelScopeInfo extends ScopeInfo {
/// this scope.
Set<ir.VariableDeclaration> freeVariables = new Set<ir.VariableDeclaration>();
- KernelScopeInfo(this.thisLocal)
+ /// Used to map [freeVariables] to their corresponding locals.
+ final KernelToLocalsMap localsMap;
+
+ KernelScopeInfo(this.thisLocal, this.localsMap)
: localsUsedInTryOrSync = new Set<Local>(),
boxedVariables = new Set<Local>();
KernelScopeInfo.from(this.thisLocal, KernelScopeInfo info)
: localsUsedInTryOrSync = info.localsUsedInTryOrSync,
- boxedVariables = info.boxedVariables;
+ boxedVariables = info.boxedVariables,
+ localsMap = info.localsMap;
- KernelScopeInfo.withBoxedVariables(this.boxedVariables, this.thisLocal)
- : localsUsedInTryOrSync = new Set<Local>();
+ KernelScopeInfo.withBoxedVariables(
+ this.boxedVariables,
+ this.localsUsedInTryOrSync,
+ this.freeVariables,
+ this.localsMap,
+ this.thisLocal);
void forEachBoxedVariable(f(Local local, FieldEntity field)) {
boxedVariables.forEach((Local l) {
@@ -215,8 +279,15 @@ class KernelScopeInfo extends ScopeInfo {
class KernelCapturedScope extends KernelScopeInfo implements CapturedScope {
final Local context;
- KernelCapturedScope(Set<Local> boxedVariables, this.context, Local thisLocal)
- : super.withBoxedVariables(boxedVariables, thisLocal);
+ KernelCapturedScope(
+ Set<Local> boxedVariables,
+ this.context,
+ Set<Local> localsUsedInTryOrSync,
+ Set<ir.VariableDeclaration> freeVariables,
+ KernelToLocalsMap localsMap,
+ Local thisLocal)
+ : super.withBoxedVariables(boxedVariables, localsUsedInTryOrSync,
+ freeVariables, localsMap, thisLocal);
bool get requiresContextBox => boxedVariables.isNotEmpty;
}
@@ -225,9 +296,16 @@ class KernelCapturedLoopScope extends KernelCapturedScope
implements CapturedLoopScope {
final List<Local> boxedLoopVariables;
- KernelCapturedLoopScope(Set<Local> boxedVariables, this.boxedLoopVariables,
- Local context, Local thisLocal)
- : super(boxedVariables, context, thisLocal);
+ KernelCapturedLoopScope(
+ Set<Local> boxedVariables,
+ this.boxedLoopVariables,
+ Local context,
+ Set<Local> localsUsedInTryOrSync,
+ Set<ir.VariableDeclaration> freeVariables,
+ KernelToLocalsMap localsMap,
+ Local thisLocal)
+ : super(boxedVariables, context, localsUsedInTryOrSync, freeVariables,
+ localsMap, thisLocal);
bool get hasBoxedLoopVariables => boxedLoopVariables.isNotEmpty;
}
@@ -235,8 +313,8 @@ class KernelCapturedLoopScope extends KernelCapturedScope
// TODO(johnniwinther): Add unittest for the computed [ClosureClass].
class KernelClosureClass extends KernelScopeInfo
implements ClosureRepresentationInfo, JClass {
- // TODO(efortuna): Generate unique name for each closure class.
- final String name = 'ClosureClass';
+ final String name;
+ final JLibrary library;
/// Index into the classData, classList and classEnvironment lists where this
/// entity is stored in [JsToFrontendMapImpl].
@@ -244,8 +322,45 @@ class KernelClosureClass extends KernelScopeInfo
final Map<Local, JField> localToFieldMap = new Map<Local, JField>();
- KernelClosureClass.fromScopeInfo(KernelScopeInfo info)
- : super.from(info.thisLocal, info);
+ KernelClosureClass.fromScopeInfo(
+ this.name, this.library, KernelScopeInfo info)
+ : super.from(info.thisLocal, info) {
+ // Make a corresponding field entity in this closure class for every single
+ // freeVariable in the KernelScopeInfo.freeVariable.
+ int i = 0;
+ for (ir.VariableDeclaration variable in info.freeVariables) {
+ // NOTE: This construction order may be slightly different than the
+ // old Element version. The old version did all the boxed items and then
+ // all the others.
+ Local capturedLocal = info.localsMap.getLocal(variable);
+ if (info.isBoxed(capturedLocal)) {
+ // TODO(efortuna): Coming soon.
+ } else {
+ localToFieldMap[capturedLocal] = new ClosureField(
+ _getClosureVariableName(capturedLocal.name, i),
+ this,
+ variable.isConst,
+ variable.isFinal || variable.isConst);
+ // TODO(efortuna): These probably need to get registered somewhere.
+ }
+ i++;
+ }
+ }
+
+ /// Generate a unique name for the [id]th closure field, with proposed name
+ /// [name].
+ ///
+ /// The result is used as the name of [ClosureFieldElement]s, and must
+ /// therefore be unique to avoid breaking an invariant in the element model
+ /// (classes cannot declare multiple fields with the same name).
+ ///
+ /// Also, the names should be distinct from real field names to prevent
+ /// clashes with selectors for those fields.
+ ///
+ /// These names are not used in generated code, just as element name.
+ String _getClosureVariableName(String name, int id) {
+ return "_captured_${name}_$id";
+ }
// TODO(efortuna): Implement.
Local get closureEntity => null;
@@ -255,36 +370,36 @@ class KernelClosureClass extends KernelScopeInfo
// TODO(efortuna): Implement.
FunctionEntity get callMethod => null;
- // TODO(efortuna): Implement.
- List<Local> get createdFieldEntities => const <Local>[];
+ List<Local> get createdFieldEntities => localToFieldMap.keys.toList();
// TODO(efortuna): Implement.
FieldEntity get thisFieldEntity => null;
- // TODO(efortuna): Implement.
- void forEachCapturedVariable(f(Local from, FieldEntity to)) {}
+ void forEachCapturedVariable(f(Local from, JField to)) {
+ localToFieldMap.forEach(f);
+ }
// TODO(efortuna): Implement.
@override
- void forEachBoxedVariable(f(Local local, FieldEntity field)) {}
+ void forEachBoxedVariable(f(Local local, JField field)) {}
// TODO(efortuna): Implement.
- void forEachFreeVariable(f(Local variable, FieldEntity field)) {}
+ void forEachFreeVariable(f(Local variable, JField field)) {}
// TODO(efortuna): Implement.
bool isVariableBoxed(Local variable) => false;
- // TODO(efortuna): Implement.
- // Why is this closure not actually a closure? Well, to properly call
- // ourselves a closure, we need to register the new closure class with the
- // ClosedWorldRefiner, which currently only takes elements. The change to
- // that (and the subsequent adjustment here) will follow soon.
- bool get isClosure => false;
+ bool get isClosure => true;
bool get isAbstract => false;
- // TODO(efortuna): Talk to Johnni.
- JLibrary get library => null;
-
String toString() => '${jsElementPrefix}class($name)';
}
+
+class ClosureField extends JField {
+ ClosureField(String name, KernelClosureClass containingClass, bool isConst,
+ bool isAssignable)
+ : super(-1, containingClass.library, containingClass,
+ new Name(name, containingClass.library),
+ isAssignable: isAssignable, isConst: isConst);
+}
« no previous file with comments | « no previous file | pkg/compiler/lib/src/js_model/closure_visitors.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698