Skip to content

Commit

Permalink
fix #159, static renames for caller/arguments
Browse files Browse the repository at this point in the history
R=jacobr@google.com

Review URL: https://codereview.chromium.org/1111803005
  • Loading branch information
John Messerly committed Apr 29, 2015
1 parent dc4b3d5 commit 56c75ff
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 21 deletions.
9 changes: 7 additions & 2 deletions pkg/dev_compiler/lib/runtime/dart_runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// 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.

var dart, _js_helper;
var dart, _js_helper, _js_primitives;
(function (dart) {
'use strict';

Expand Down Expand Up @@ -760,7 +760,9 @@ var dart, _js_helper;
let initMethod = proto[name];
let ctor = function() { return initMethod.apply(this, arguments); }
ctor.prototype = proto;
clazz[name] = ctor;
// Use defineProperty so we don't hit a property defined on Function,
// like `caller` and `arguments`.
defineProperty(clazz, name, { value: ctor, configurable: true });
}
dart.defineNamedConstructor = defineNamedConstructor;

Expand Down Expand Up @@ -937,4 +939,7 @@ var dart, _js_helper;
_js_helper = _js_helper || {};
_js_helper.checkNum = notNull;

_js_primitives = _js_primitives || {};
_js_primitives.printString = (s) => console.log(s);

})(dart || (dart = {}));
17 changes: 4 additions & 13 deletions pkg/dev_compiler/lib/src/codegen/js_codegen.dart
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
for (FieldDeclaration member in staticFields) {
for (VariableDeclaration field in member.fields.variables) {
var fieldName = field.name.name;
if (field.isConst || _isFieldInitConstant(field)) {
if ((field.isConst || _isFieldInitConstant(field)) &&
!JS.invalidStaticFieldName(fieldName)) {
var init = _visit(field.initializer);
if (init == null) init = new JS.LiteralNull();
body.add(js.statement('#.# = #;', [name, fieldName, init]));
Expand Down Expand Up @@ -2363,18 +2364,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
JS.Expression _emitMemberName(String name,
{DartType type, bool unary: false, bool isStatic: false}) {

// Static methods skip most of the rename steps.
if (isStatic) {
if (JS.invalidStaticMethodName(name)) {
// Choose an string name. Use an invalid identifier so it won't conflict
// with any valid member names.
// TODO(jmesserly): this works around the problem, but I'm pretty sure we
// don't need it, as static methods seemed to work. The only concrete
// issue we saw was in the defineNamedConstructor helper function.
name = '$name*';
}
return _propertyName(name);
}
// Static members skip the rename steps.
if (isStatic) return _propertyName(name);

if (name.startsWith('_')) {
return _privateNames.putIfAbsent(
Expand Down
4 changes: 2 additions & 2 deletions pkg/dev_compiler/lib/src/codegen/js_names.dart
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,9 @@ bool invalidVariableName(String keyword, {bool strictMode: true}) {
return false;
}

/// Returns true for invalid static method names in strict mode.
/// Returns true for invalid static field names in strict mode.
/// In particular, "caller" "callee" and "arguments" cannot be used.
bool invalidStaticMethodName(String name) {
bool invalidStaticFieldName(String name) {
switch (name) {
case "arguments":
case "caller":
Expand Down
21 changes: 17 additions & 4 deletions pkg/dev_compiler/test/codegen/expect/names.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,38 @@ var names;
return 456;
}
class Frame extends core.Object {
['caller*'](arguments$) {
caller(arguments$) {
this.arguments = arguments$;
}
static ['callee*']() {
static callee() {
return null;
}
}
dart.defineNamedConstructor(Frame, 'caller*');
dart.defineNamedConstructor(Frame, 'caller');
class Frame2 extends core.Object {}
dart.defineLazyProperties(Frame2, {
get caller() {
return 100;
},
set caller(_) {},
get arguments() {
return 200;
},
set arguments(_) {}
});
// Function main: () → dynamic
function main() {
core.print(exports.exports);
core.print(new Foo()[_foo$]());
core.print(_foo());
core.print(new Frame.caller([1, 2, 3]));
let eval$ = dart.bind(Frame, 'callee*');
let eval$ = dart.bind(Frame, 'callee');
core.print(eval$);
core.print(dart.notNull(Frame2.caller) + dart.notNull(Frame2.arguments));
}
// Exports:
exports.Foo = Foo;
exports.Frame = Frame;
exports.Frame2 = Frame2;
exports.main = main;
})(names || (names = {}));
7 changes: 7 additions & 0 deletions pkg/dev_compiler/test/codegen/names.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,18 @@ class Frame {
static callee() => null;
}


class Frame2 {
static int caller = 100;
static int arguments = 200;
}

main() {
print(exports);
print(new Foo()._foo());
print(_foo());
print(new Frame.caller([1,2,3]));
var eval = Frame.callee;
print(eval);
print(Frame2.caller + Frame2.arguments);
}
10 changes: 10 additions & 0 deletions pkg/dev_compiler/test/codegen/names.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>Names test</title>
</head>
<body>
<script type="application/dart" src="names.dart"></script>
</body>
</html>

0 comments on commit 56c75ff

Please sign in to comment.