Skip to content

Commit

Permalink
fix a few more codegen issues:
Browse files Browse the repository at this point in the history
* don't generate script tag
* handle U+2028 and U+2029
* handle name conflict with constructor

[email protected]

Review URL: https://codereview.chromium.org/1347453002 .
  • Loading branch information
John Messerly committed Sep 15, 2015
1 parent 637c2c8 commit ff1b78c
Show file tree
Hide file tree
Showing 10 changed files with 166 additions and 18 deletions.
4 changes: 4 additions & 0 deletions pkg/dev_compiler/lib/runtime/_classes.js
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,10 @@ dart_library.library('dart_runtime/_classes', null, /* Imports */[

function canonicalMember(obj, name) {
if (obj != null && obj[_extensionType]) return dartx[name];
// Check for certain names that we can't use in JS
if (name == 'constructor' || name == 'prototype') {
name = '+' + name;
}
return name;
}
exports.canonicalMember = canonicalMember;
Expand Down
6 changes: 2 additions & 4 deletions pkg/dev_compiler/lib/runtime/_types.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ dart_library.library('dart_runtime/_types', null, /* Imports */[
const getOwnPropertyNames = Object.getOwnPropertyNames;

const assert = dart_utils.assert;
const copyProperties = dart_utils.copyProperties;
const safeGetOwnProperty = dart_utils.safeGetOwnProperty;

/**
* Types in dart are represented at runtime as follows.
Expand Down Expand Up @@ -64,13 +62,13 @@ dart_library.library('dart_runtime/_types', null, /* Imports */[

class Bottom extends TypeRep {
toString() { return "bottom"; }
};
}
let bottomR = new Bottom();
exports.bottom = bottomR;

class JSObject extends TypeRep {
toString() { return "NativeJavaScriptObject"; }
};
}
let jsobjectR = new JSObject();
exports.jsobject = jsobjectR;

Expand Down
16 changes: 11 additions & 5 deletions pkg/dev_compiler/lib/src/codegen/js_codegen.dart
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,13 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
module
]);

var jsBin = compiler.options.runnerOptions.v8Binary;

String scriptTag = null;
if (library.library.scriptTag != null) scriptTag = '/usr/bin/env $jsBin';
return new JS.Program(<JS.Statement>[moduleDef], scriptTag: scriptTag);
// TODO(jmesserly): scriptTag support.
// Enable this if we know we're targetting command line environment?
// It doesn't work in browser.
// var jsBin = compiler.options.runnerOptions.v8Binary;
// String scriptTag = null;
// if (library.library.scriptTag != null) scriptTag = '/usr/bin/env $jsBin';
return new JS.Program(<JS.Statement>[moduleDef]);
}

void _emitModuleItem(AstNode node) {
Expand Down Expand Up @@ -3191,6 +3193,10 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
name = 'set';
} else if (name == '-' && unary) {
name = 'unary-';
} else if (name == 'constructor' || name == 'prototype') {
// This uses an illegal (in Dart) character for a member, avoiding the
// conflict. We could use practically any character for this.
name = '+$name';
}

// Dart "extension" methods. Used for JS Array, Boolean, Number, String.
Expand Down
14 changes: 11 additions & 3 deletions pkg/dev_compiler/lib/src/js/builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -298,17 +298,25 @@ class JsBuilder {
LiteralString escapedString(String value, [String quote = '"']) {
// Start by escaping the backslashes.
String escaped = value.replaceAll('\\', '\\\\');
// Do not escape unicode characters and ' because they are allowed in the
// string literal anyway.
var re = new RegExp('\n|\r|$quote|\b|\f|\t|\v');

// http://www.ecma-international.org/ecma-262/6.0/#sec-literals-string-literals
// > All code points may appear literally in a string literal except for the
// > closing quote code points, U+005C (REVERSE SOLIDUS),
// > U+000D (CARRIAGE RETURN), U+2028 (LINE SEPARATOR),
// > U+2029 (PARAGRAPH SEPARATOR), and U+000A (LINE FEED).
var re = new RegExp('\n|\r|$quote|\b|\f|\t|\v|\u2028|\u2029');
escaped = escaped.replaceAllMapped(re, (m) {
switch (m.group(0)) {
case "\n" : return r"\n";
case "\r" : return r"\r";
case "\u2028": return r"\u2028";
case "\u2029": return r"\u2029";
// Quotes are only replaced if they conflict with the containing quote
case '"': return r'\"';
case "'": return r"\'";
case "`": return r"\`";
// TODO(jmesserly): these don't need to be escaped for correctness,
// but they are conventionally escaped.
case "\b" : return r"\b";
case "\t" : return r"\t";
case "\f" : return r"\f";
Expand Down
9 changes: 7 additions & 2 deletions pkg/dev_compiler/test/browser/language_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,18 @@
}
}

suite('scoping', () => {
suite('scopes and names', () => {
dartLanguageTests([
'implicit_scope_test',
['built_in_identifier_test', 'none', 1]
['built_in_identifier_test', 'none', 1],
'naming3_test'
]);
});

suite('strings', () => {
dartLanguageTests(['optimized_string_charcodeat_test']);
});

suite('method binding', () => {
dartLanguageTests([
['super_bound_closure_test', 'none'],
Expand Down
4 changes: 2 additions & 2 deletions pkg/dev_compiler/test/codegen/expect/html_input.html
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
<script src="dev_compiler/runtime/dart/mirrors.js"></script>
<script src="dev_compiler/runtime/dart/_js_mirrors.js"></script>
<script src="dev_compiler/runtime/dart/js.js"></script>
<script src="dir/html_input_e.js"></script>
<script src="dir/html_input_c.js"></script>
<script src="dir/html_input_d.js"></script>
<script src="dir/html_input_b.js"></script>
<script src="dir/html_input_e.js"></script>
<script src="dir/html_input_c.js"></script>
<script src="dir/html_input_a.js"></script>
<script>dart_library.start('dir/html_input_a');</script>

Expand Down
1 change: 0 additions & 1 deletion pkg/dev_compiler/test/codegen/expect/script.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#!/usr/bin/env iojs
dart_library.library('script', null, /* Imports */[
"dart_runtime/dart",
'dart/core'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ <h1>drfibonacci's Sunflower Spectacular</h1>
<script src="../dev_compiler/runtime/dart/mirrors.js"></script>
<script src="../dev_compiler/runtime/dart/_js_mirrors.js"></script>
<script src="../dev_compiler/runtime/dart/js.js"></script>
<script src="circle.js"></script>
<script src="dom.js"></script>
<script src="circle.js"></script>
<script src="painter.js"></script>
<script src="sunflower.js"></script>
<script>dart_library.start('sunflower/sunflower');</script>
Expand Down
47 changes: 47 additions & 0 deletions pkg/dev_compiler/test/codegen/language/naming3_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// 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 "package:expect/expect.dart";

class A {
var __PROTO__ = 499;
var constructor = 1;
var prototype = 2;
}

// TODO(jmesserly): changed this test to avoid shadowing a field with a getter,
// which DDC doesn't currently support, see:
// https://github.com/dart-lang/dev_compiler/issues/52
class A2 {
get __PROTO__ => 499;
get constructor => 1;
get prototype => 2;
}

class B extends A2 {
get __PROTO__ => 42;
get constructor => 3;
get prototype => 4;
}

main() {
var a = new A();
var a2 = new A2();
var b = new B();
var list = [a, a2, b];
for (int i = 0; i < list.length; i++) {
var proto = list[i].__PROTO__;
var constructor = list[i].constructor;
var prototype = list[i].prototype;
if (i < 2) {
Expect.equals(499, proto);
Expect.equals(1, constructor);
Expect.equals(2, prototype);
} else {
Expect.equals(42, proto);
Expect.equals(3, constructor);
Expect.equals(4, prototype);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// 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.
// VMOptions=--optimization-counter-threshold=10 --no-use-osr

// Test optimized CodeUnitAt and array access.

import "package:expect/expect.dart";


String one_byte = "hest";
String two_byte = "h\u{2029}ns";

int testOneByteCodeUnitAt(String x, int j) {
int test() {
return x.codeUnitAt(j);
}
for (int i = 0; i < 20; i++) test();
return test();
}


int testTwoByteCodeUnitAt(String x, int j) {
int test() {
return x.codeUnitAt(j);
}
for (int i = 0; i < 20; i++) test();
return test();
}


int testConstantStringCodeUnitAt(int j) {
int test() {
return "høns".codeUnitAt(j);
}
for (int i = 0; i < 20; i++) test();
return test();
}


int testConstantIndexCodeUnitAt(String x) {
int test() {
return x.codeUnitAt(1);
}
for (int i = 0; i < 20; i++) test();
return test();
}


int testOneByteCodeUnitAtInLoop(var x) {
var result = 0;
for (int i = 0; i < x.length; i++) {
result += x.codeUnitAt(i);
}
return result;
}


int testTwoByteCodeUnitAtInLoop(var x) {
var result = 0;
for (int i = 0; i < x.length; i++) {
result += x.codeUnitAt(i);
}
return result;
}


main() {
for (int j = 0; j < 10; j++) {
Expect.equals(101, testOneByteCodeUnitAt(one_byte, 1));
Expect.equals(8233, testTwoByteCodeUnitAt(two_byte, 1));
Expect.equals(248, testConstantStringCodeUnitAt(1));
Expect.equals(101, testConstantIndexCodeUnitAt(one_byte));
}
for (int j = 0; j < 20; j++) {
Expect.equals(436, testOneByteCodeUnitAtInLoop(one_byte));
Expect.equals(8562, testTwoByteCodeUnitAtInLoop(two_byte));
}
Expect.throws(() => testOneByteCodeUnitAtInLoop(123));
Expect.throws(() => testTwoByteCodeUnitAtInLoop(123));
}

0 comments on commit ff1b78c

Please sign in to comment.