Skip to content

Commit

Permalink
Use lazy static fields when overriding static getters or setters.
Browse files Browse the repository at this point in the history
Fixes #522

R=jmesserly@google.com

Review URL: https://codereview.chromium.org/1927353002 .
  • Loading branch information
harryterkelsen committed Apr 28, 2016
1 parent 8c745c6 commit d109467
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 19 deletions.
50 changes: 31 additions & 19 deletions pkg/dev_compiler/lib/src/compiler/code_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -546,13 +546,14 @@ class CodeGenerator extends GeneralizingAstVisitor
className = _emitTopLevelName(classElem);
}

var allFields = new List.from(fields)..addAll(staticFields);
var superclasses = getSuperclasses(classElem);
var virtualFields = <FieldElement, JS.TemporaryId>{};
var virtualFieldSymbols = <JS.Statement>[];
_registerVirtualFields(classElem, className, superclasses, fields,
virtualFields, virtualFieldSymbols);
var staticFieldOverrides = new HashSet<FieldElement>();
_registerPropertyOverrides(classElem, className, superclasses, allFields,
virtualFields, virtualFieldSymbols, staticFieldOverrides);

var allFields = new List.from(fields)..addAll(staticFields);
var classExpr = _emitClassExpression(classElem,
_emitClassMethods(node, ctors, fields, superclasses, virtualFields),
fields: allFields);
Expand All @@ -579,30 +580,35 @@ class CodeGenerator extends GeneralizingAstVisitor
}

body = <JS.Statement>[classDef];
_emitStaticFields(staticFields, classElem, body);
_emitStaticFields(staticFields, staticFieldOverrides, classElem, body);
_registerExtensionType(classElem, body);
return _statement(body);
}

void _registerVirtualFields(
void _registerPropertyOverrides(
ClassElement classElem,
JS.Expression className,
List<ClassElement> superclasses,
List<FieldDeclaration> fields,
Map<FieldElement, JS.TemporaryId> virtualFields,
List<JS.Statement> virtualFieldSymbols) {
List<JS.Statement> virtualFieldSymbols,
Set<FieldElement> staticFieldOverrides) {
for (var field in fields) {
for (VariableDeclaration field in field.fields.variables) {
var overrideInfo = checkForPropertyOverride(
field.element, superclasses, _extensionTypes);
if (overrideInfo.foundGetter || overrideInfo.foundSetter) {
var fieldName =
_emitMemberName(field.element.name, type: classElem.type);
var virtualField = new JS.TemporaryId(field.element.name);
virtualFields[field.element] = virtualField;
virtualFieldSymbols.add(js.statement(
'const # = Symbol(#.name + "." + #.toString());',
[virtualField, className, fieldName]));
if (field.element.isStatic) {
staticFieldOverrides.add(field.element);
} else {
var fieldName =
_emitMemberName(field.element.name, type: classElem.type);
var virtualField = new JS.TemporaryId(field.element.name);
virtualFields[field.element] = virtualField;
virtualFieldSymbols.add(js.statement(
'const # = Symbol(#.name + "." + #.toString());',
[virtualField, className, fieldName]));
}
}
}
}
Expand Down Expand Up @@ -1012,12 +1018,16 @@ class CodeGenerator extends GeneralizingAstVisitor

/// Emits static fields for a class, and initialize them eagerly if possible,
/// otherwise define them as lazy properties.
void _emitStaticFields(List<FieldDeclaration> staticFields,
ClassElement classElem, List<JS.Statement> body) {
void _emitStaticFields(
List<FieldDeclaration> staticFields,
Set<FieldElement> staticFieldOverrides,
ClassElement classElem,
List<JS.Statement> body) {
var lazyStatics = <VariableDeclaration>[];
for (FieldDeclaration member in staticFields) {
for (VariableDeclaration field in member.fields.variables) {
JS.Statement eagerField = _emitConstantStaticField(classElem, field);
JS.Statement eagerField =
_emitConstantStaticField(classElem, field, staticFieldOverrides);
if (eagerField != null) {
body.add(eagerField);
} else {
Expand Down Expand Up @@ -2573,8 +2583,8 @@ class CodeGenerator extends GeneralizingAstVisitor
/// Otherwise, we'll need to generate a lazy-static field. That ensures
/// correct visible behavior, as well as avoiding referencing something that
/// isn't defined yet (because it is defined later in the module).
JS.Statement _emitConstantStaticField(
ClassElement classElem, VariableDeclaration field) {
JS.Statement _emitConstantStaticField(ClassElement classElem,
VariableDeclaration field, Set<FieldElement> staticFieldOverrides) {
PropertyInducingElement element = field.element;
assert(element.isStatic);

Expand All @@ -2586,7 +2596,9 @@ class CodeGenerator extends GeneralizingAstVisitor
isLoaded && (field.isConst || _constField.isFieldInitConstant(field));

var fieldName = field.name.name;
if (eagerInit && !JS.invalidStaticFieldName(fieldName)) {
if (eagerInit &&
!JS.invalidStaticFieldName(fieldName) &&
!staticFieldOverrides.contains(element)) {
return annotate(
js.statement('#.# = #;', [
_emitTopLevelName(classElem),
Expand Down
4 changes: 4 additions & 0 deletions pkg/dev_compiler/lib/src/compiler/js_field_storage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ PropertyOverrideResult checkForPropertyOverride(FieldElement field,
var superprop = getProperty(superclass, field.library, field.name);
if (superprop == null) continue;

// Static fields can override superclass static fields. However, we need to
// handle the case where they override a getter or setter.
if (field.isStatic && !superprop.isSynthetic) continue;

var getter = superprop.getter;
bool hasGetter = getter != null && !getter.isAbstract;
if (hasGetter) foundGetter = true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) 2016, 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 'package:expect/expect.dart';

class Foo {
static int get x => 42;
}

class Bar extends Foo {
static int x = 12;
}

void main() {
Expect.equals(12, Bar.x);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) 2016, 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 'package:expect/expect.dart';

class Foo {
static int get x => 42;
static void set x(value) {}
}

class Bar extends Foo {
static int x = 12;
}

void main() {
Expect.equals(12, Bar.x);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) 2016, 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 'package:expect/expect.dart';

class Foo {
static int x = 42;
}

class Bar extends Foo {
static int x = 12;
}

void main() {
Expect.equals(12, Bar.x);
}

0 comments on commit d109467

Please sign in to comment.