Skip to content

Commit

Permalink
[refactor] js_codegen: remove currentClass, simplify visitSimpleIdent…
Browse files Browse the repository at this point in the history 
…ifier

We were tracking currentClass for no real reason.

Also attempted to discern what visitSimpleIdentifier was actually doing. It now prefers the original element (the "field" not the synthetic getter/setter) whenever possible. Previously was a mismash.

This little change tripped on the (larger, not fixed yet) issue #138 ... but I think I found a way around the issue for now, by tweaking the private TypeDataArray class.

R=jacobr@google.com

Review URL: https://codereview.chromium.org/1095563003
  • Loading branch information
John Messerly committed Apr 21, 2015
1 parent 9efc804 commit 1e43b3a
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 36 deletions.
9 changes: 6 additions & 3 deletions pkg/dev_compiler/lib/runtime/dart/_native_typed_data.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,9 +582,6 @@ var _native_typed_data;
dart.defineNamedConstructor(NativeByteData, 'view');
let _setRangeFast = Symbol('_setRangeFast');
class NativeTypedArray extends NativeTypedData {
get length() {
return dart.as(this.length, core.int);
}
[_setRangeFast](start, end, source, skipCount) {
let targetLength = this.length;
this[_checkIndex](start, dart.notNull(targetLength) + 1);
Expand All @@ -606,6 +603,9 @@ var _native_typed_data;
}
NativeTypedArray[dart.implements] = () => [_js_helper.JavaScriptIndexingBehavior];
class NativeTypedArrayOfDouble extends dart.mixin(NativeTypedArray, collection.ListMixin$(core.double), _internal.FixedLengthListMixin$(core.double)) {
get [core.$length]() {
return dart.as(this.length, core.int);
}
[core.$get](index) {
this[_checkIndex](index, this[core.$length]);
return this[index];
Expand All @@ -625,6 +625,9 @@ var _native_typed_data;
}
}
class NativeTypedArrayOfInt extends dart.mixin(NativeTypedArray, collection.ListMixin$(core.int), _internal.FixedLengthListMixin$(core.int)) {
get [core.$length]() {
return dart.as(this.length, core.int);
}
[core.$set](index, value) {
this[_checkIndex](index, this[core.$length]);
this[index] = value;
Expand Down
60 changes: 29 additions & 31 deletions pkg/dev_compiler/lib/src/codegen/js_codegen.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
/// The variable for the current catch clause
SimpleIdentifier _catchParameter;

ClassDeclaration currentClass;
ConstantEvaluator _constEvaluator;

final _exports = new Set<String>();
Expand Down Expand Up @@ -319,8 +318,6 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
var jsName = getAnnotationValue(node, _isJsNameAnnotation);
if (jsName != null) return _emitJsType(node.name.name, jsName);

currentClass = node;

var ctors = <ConstructorDeclaration>[];
var fields = <FieldDeclaration>[];
var staticFields = <FieldDeclaration>[];
Expand All @@ -337,7 +334,6 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {

var body =
_finishClassMembers(classElem, classExpr, ctors, fields, staticFields);
currentClass = null;

return _finishClassDef(type, body);
}
Expand Down Expand Up @@ -1016,49 +1012,55 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
/// going through the qualified library name if necessary.
@override
JS.Expression visitSimpleIdentifier(SimpleIdentifier node) {
var e = node.staticElement;
if (e == null) {
var accessor = node.staticElement;
if (accessor == null) {
return js.commentExpression(
'Unimplemented unknown name', new JS.Identifier(node.name));
}

var variable = e is PropertyAccessorElement ? e.variable : e;
var name = variable.name;
// Get the original declaring element. If we had a property accessor, this
// indirects back to a (possibly synthetic) field.
var element = accessor;
if (element is PropertyAccessorElement) element = accessor.variable;
var name = element.name;

// library member
if (e.enclosingElement is CompilationUnitElement &&
(e.library != libraryInfo.library ||
variable is TopLevelVariableElement && !variable.isConst)) {
return js.call('#.#', [_libraryName(e.library), name]);
if (element.enclosingElement is CompilationUnitElement &&
(element.library != libraryInfo.library ||
element is TopLevelVariableElement && !element.isConst)) {
return js.call('#.#', [_libraryName(element.library), name]);
}

// instance member
if (currentClass != null && _needsImplicitThis(e)) {
return js.call(
'this.#', _emitMemberName(name, type: currentClass.element.type));
}
// Unqualified class member. This could mean implicit-this, or implicit
// call to a static from the same class.
if (element is ClassMemberElement && element is! ConstructorElement) {
bool isStatic = element.isStatic;
var type = element.enclosingElement.type;

// static member
if (e is ExecutableElement &&
e.isStatic &&
variable.enclosingElement is ClassElement) {
var className = (variable.enclosingElement as ClassElement).name;
return js.call('#.#', [className, _emitMemberName(name, isStatic: true)]);
// For instance methods, we add implicit-this.
// For static methods, we add the raw type name, without generics or
// library prefix. We don't need those because static calls can't use
// the generic type.
var target = isStatic ? new JS.Identifier(type.name) : new JS.This();
var member = _emitMemberName(name, isStatic: isStatic, type: type);
return new JS.PropertyAccess(target, member);
}

// initializing formal parameter, e.g. `Point(this.x)`
if (e is ParameterElement && e.isInitializingFormal && e.isPrivate) {
if (element is ParameterElement &&
element.isInitializingFormal &&
element.isPrivate) {
/// Rename private names so they don't shadow the private field symbol.
/// The renamer would handle this, but it would prefer to rename the
/// temporary used for the private symbol. Instead rename the parameter.
return _getTemp(e, '${name.substring(1)}');
return _getTemp(element, '${name.substring(1)}');
}

if (_isTemporary(e)) {
if (_isTemporary(element)) {
if (name[0] == '#') {
return new JS.InterpolatedExpression(name.substring(1));
} else {
return _getTemp(e, name);
return _getTemp(element, name);
}
}

Expand Down Expand Up @@ -2312,10 +2314,6 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
}

DartType getStaticType(Expression e) => rules.getStaticType(e);

static bool _needsImplicitThis(Element e) =>
e is PropertyAccessorElement && !e.variable.isStatic ||
e is ClassMemberElement && !e.isStatic && e is! ConstructorElement;
}

class JSGenerator extends CodeGenerator {
Expand Down
7 changes: 6 additions & 1 deletion pkg/dev_compiler/test/generated_sdk/lib/_internal/compiler/js_lib/native_typed_data.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,8 @@ class NativeByteData extends NativeTypedData implements ByteData {

abstract class NativeTypedArray extends NativeTypedData
implements JavaScriptIndexingBehavior {
int get length => JS('JSUInt32', '#.length', this);
// TODO(jmesserly): moved `length` to subclass to (somewhat) mitigate
// <https://github.com/dart-lang/dev_compiler/issues/138>

void _setRangeFast(int start, int end,
NativeTypedArray source, int skipCount) {
Expand Down Expand Up @@ -887,6 +888,8 @@ abstract class NativeTypedArrayOfDouble
extends NativeTypedArray
with ListMixin<double>, FixedLengthListMixin<double> {

int get length => JS('JSUInt32', '#.length', this);

num operator[](int index) {
_checkIndex(index, length);
return JS('num', '#[#]', this, index);
Expand All @@ -912,6 +915,8 @@ abstract class NativeTypedArrayOfInt
with ListMixin<int>, FixedLengthListMixin<int>
implements List<int> {

int get length => JS('JSUInt32', '#.length', this);

// operator[]() is not here since different versions have different return
// types

Expand Down
7 changes: 6 additions & 1 deletion pkg/dev_compiler/tool/input_sdk/private/native_typed_data.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,8 @@ class NativeByteData extends NativeTypedData implements ByteData {

abstract class NativeTypedArray extends NativeTypedData
implements JavaScriptIndexingBehavior {
int get length => JS('JSUInt32', '#.length', this);
// TODO(jmesserly): moved `length` to subclass to (somewhat) mitigate
// <https://github.com/dart-lang/dev_compiler/issues/138>

void _setRangeFast(int start, int end,
NativeTypedArray source, int skipCount) {
Expand Down Expand Up @@ -887,6 +888,8 @@ abstract class NativeTypedArrayOfDouble
extends NativeTypedArray
with ListMixin<double>, FixedLengthListMixin<double> {

int get length => JS('JSUInt32', '#.length', this);

num operator[](int index) {
_checkIndex(index, length);
return JS('num', '#[#]', this, index);
Expand All @@ -912,6 +915,8 @@ abstract class NativeTypedArrayOfInt
with ListMixin<int>, FixedLengthListMixin<int>
implements List<int> {

int get length => JS('JSUInt32', '#.length', this);

// operator[]() is not here since different versions have different return
// types

Expand Down

0 comments on commit 1e43b3a

Please sign in to comment.