Skip to content

Commit

Permalink
Redo how Type objects are exposed from DDC.
Browse files Browse the repository at this point in the history
- Instead of using the raw runtime type that DDC uses for its type
  checks, use a WrappedType that correctly implements Type's interface.
- Compile class literals to wrap the type in a WrappedType.
- Make Object.runtimeType() do the same thing.

Fixes #488. Fixes #511.

[email protected]

Review URL: https://codereview.chromium.org/1944483002 .
  • Loading branch information
munificent committed May 3, 2016
1 parent 6485751 commit 44db042
Show file tree
Hide file tree
Showing 16 changed files with 299 additions and 172 deletions.
9 changes: 0 additions & 9 deletions pkg/dev_compiler/lib/runtime/dart_library.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,6 @@ var dart_library =

// Force import of core.
var dart_sdk = import_('dart_sdk');
var core = dart_sdk.core;

// TODO(jmesserly): this can't be right.
// See: https://github.com/dart-lang/dev_compiler/issues/488
core.Object.toString = function() {
// Interface types are represented by the corresponding constructor
// function. This ensures that Dart interface types print properly.
return this.name;
}

// TODO(vsm): DOM facades?
// See: https://github.com/dart-lang/dev_compiler/issues/173
Expand Down
141 changes: 81 additions & 60 deletions pkg/dev_compiler/lib/runtime/dart_sdk.js

Large diffs are not rendered by default.

29 changes: 27 additions & 2 deletions pkg/dev_compiler/lib/src/compiler/code_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ class CodeGenerator extends GeneralizingAstVisitor

String _buildRoot;

/// Whether we are currently generating code for the body of a `JS()` call.
bool _isInForeignJS = false;

CodeGenerator(AnalysisContext c, this.options, this._extensionTypes)
: context = c,
types = c.typeProvider,
Expand Down Expand Up @@ -219,7 +222,6 @@ class CodeGenerator extends GeneralizingAstVisitor
// Add implicit dart:core dependency so it is first.
emitLibraryName(dartCoreLibrary);

//
// Visit each compilation unit and emit its code.
//
// NOTE: declarations are not necessarily emitted in this order.
Expand Down Expand Up @@ -2009,7 +2011,18 @@ class CodeGenerator extends GeneralizingAstVisitor

// type literal
if (element is TypeDefiningElement) {
return _emitTypeName(fillDynamicTypeArgs(element.type));
var typeName = _emitTypeName(fillDynamicTypeArgs(element.type));

// If the type is a type literal expression in Dart code, wrap the raw
// runtime type in a "Type" instance.
if (!_isInForeignJS &&
node.parent is! MethodInvocation &&
node.parent is! PrefixedIdentifier &&
node.parent is! PropertyAccess) {
typeName = js.call('dart.wrapType(#)', typeName);
}

return typeName;
}

// library member
Expand Down Expand Up @@ -2477,8 +2490,20 @@ class CodeGenerator extends GeneralizingAstVisitor
source = (code as StringLiteral).stringValue;
}

// TODO(rnystrom): The JS() calls are almost never nested, and probably
// really shouldn't be, but there are at least a couple of calls in the
// HTML library where an argument to JS() is itself a JS() call. If those
// go away, this can just assert(!_isInForeignJS).
// Inside JS(), type names evaluate to the raw runtime type, not the
// wrapped Type object.
var wasInForeignJS = _isInForeignJS;
_isInForeignJS = true;

var template = js.parseForeignJS(source);
var result = template.instantiate(_visitList(templateArgs));

_isInForeignJS = wasInForeignJS;

// `throw` is emitted as a statement by `parseForeignJS`.
assert(result is JS.Expression || node.parent is ExpressionStatement);
return result;
Expand Down
58 changes: 25 additions & 33 deletions pkg/dev_compiler/test/browser/runtime_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ suite('instanceOf', () => {
let cast = dart.as;
let instanceOf = dart.is;
let strongInstanceOf = dart.strongInstanceOf;
let runtimeType = dart.realRuntimeType;
let getReifiedType = dart.getReifiedType;
let functionType = dart.functionType;
let typedef = dart.typedef;
let isSubtype = dart.isSubtype;
Expand Down Expand Up @@ -213,7 +213,7 @@ suite('instanceOf', () => {

test('int', () => {
expect(isGroundType(int), true);
expect(isGroundType(runtimeType(5)), true);
expect(isGroundType(getReifiedType(5)), true);

checkType(5, int);
checkType(5, dynamic);
Expand Down Expand Up @@ -260,7 +260,7 @@ suite('instanceOf', () => {

test('String', () => {
expect(isGroundType(String), true);
expect(isGroundType(runtimeType("foo")), true);
expect(isGroundType(getReifiedType("foo")), true);
checkType("foo", String);
checkType("foo", Object);
checkType("foo", dynamic);
Expand All @@ -278,15 +278,15 @@ suite('instanceOf', () => {


expect(isGroundType(Map), true);
expect(isGroundType(runtimeType(m1)), false);
expect(isGroundType(getReifiedType(m1)), false);
expect(isGroundType(Map$(String, String)), false);
expect(isGroundType(runtimeType(m2)), true);
expect(isGroundType(getReifiedType(m2)), true);
expect(isGroundType(Map$(Object, Object)), true);
expect(isGroundType(runtimeType(m3)), true);
expect(isGroundType(getReifiedType(m3)), true);
expect(isGroundType(Map), true);
expect(isGroundType(runtimeType(m4)), true);
expect(isGroundType(getReifiedType(m4)), true);
expect(isGroundType(collection.HashMap$(dynamic, dynamic)), true);
expect(isGroundType(runtimeType(m5)), true);
expect(isGroundType(getReifiedType(m5)), true);
expect(isGroundType(collection.LinkedHashMap), true);
expect(isGroundType(collection.LinkedHashMap), true);

Expand All @@ -295,15 +295,15 @@ suite('instanceOf', () => {
checkType(m1, Object);

// Instance of self
checkType(m1, runtimeType(m1));
checkType(m1, getReifiedType(m1));
checkType(m1, Map$(String, String));

// Covariance on generics
checkType(m1, runtimeType(m2));
checkType(m1, getReifiedType(m2));
checkType(m1, Map$(Object, Object));

// No contravariance on generics.
checkType(m2, runtimeType(m1), false, true);
checkType(m2, getReifiedType(m1), false, true);
checkType(m2, Map$(String, String), false, true);

// null is! Map
Expand Down Expand Up @@ -358,19 +358,19 @@ suite('instanceOf', () => {

test('generic and inheritance', () => {
let aaraw = new AA();
let aarawtype = runtimeType(aaraw);
let aarawtype = getReifiedType(aaraw);
let aadynamic = new (AA$(dynamic, dynamic))();
let aadynamictype = runtimeType(aadynamic);
let aadynamictype = getReifiedType(aadynamic);
let aa = new (AA$(String, List))();
let aatype = runtimeType(aa);
let aatype = getReifiedType(aa);
let bb = new (BB$(String, List))();
let bbtype = runtimeType(bb);
let bbtype = getReifiedType(bb);
let cc = new CC();
let cctype = runtimeType(cc);
let cctype = getReifiedType(cc);
// We don't allow constructing bad types.
// This was AA<String> in Dart (wrong number of type args).
let aabad = new (AA$(dart.dynamic, dart.dynamic))();
let aabadtype = runtimeType(aabad);
let aabadtype = getReifiedType(aabad);

expect(isGroundType(aatype), false);
expect(isGroundType(AA$(String, List)), false);
Expand Down Expand Up @@ -471,18 +471,18 @@ suite('instanceOf', () => {
checkType(cls7, Foo);
checkType(bar7, functionType(B, [B, String]));
checkType(cls7, functionType(B, [B, String]));
checkType(bar7, runtimeType(bar6));
checkType(cls7, runtimeType(bar6));
checkType(bar7, getReifiedType(bar6));
checkType(cls7, getReifiedType(bar6));
checkType(bar8, Foo);
checkType(cls8, Foo);
checkType(bar8, functionType(B, [B, String]));
checkType(cls8, functionType(B, [B, String]));
checkType(bar8, runtimeType(bar6), false, true);
checkType(cls8, runtimeType(bar6), false, true);
checkType(bar7, runtimeType(bar8), false, true);
checkType(cls7, runtimeType(bar8), false, true);
checkType(bar8, runtimeType(bar7), false, true);
checkType(cls8, runtimeType(bar7), false, true);
checkType(bar8, getReifiedType(bar6), false, true);
checkType(cls8, getReifiedType(bar6), false, true);
checkType(bar7, getReifiedType(bar8), false, true);
checkType(cls7, getReifiedType(bar8), false, true);
checkType(bar8, getReifiedType(bar7), false, true);
checkType(cls8, getReifiedType(bar7), false, true);

// Parameterized typedefs
expect(isGroundType(FuncG), true);
Expand Down Expand Up @@ -747,8 +747,6 @@ suite('instanceOf', () => {
assert.equal(nullHash, 0);
let nullString = dart.toString(null);
assert.equal(nullString, 'null');
let nullType = dart.runtimeType(null);
assert.equal(nullType, core.Null);

let map = new Map();
let mapHash = dart.hashCode(map);
Expand All @@ -758,8 +756,6 @@ suite('instanceOf', () => {
let mapString = dart.toString(map);
assert.equal(mapString, map.toString());
checkType(mapString, core.String);
let mapType = dart.runtimeType(map);
assert.equal(mapType, map.runtimeType);

let str = "A string";
let strHash = dart.hashCode(str);
Expand All @@ -768,17 +764,13 @@ suite('instanceOf', () => {
let strString = dart.toString(str);
checkType(strString, core.String);
assert.equal(str, strString);
let strType = dart.runtimeType(str);
assert.equal(strType, core.String);

let n = 42;
let intHash = dart.hashCode(n);
checkType(intHash, core.int);

let intString = dart.toString(n);
assert.equal(intString, '42');
let intType = dart.runtimeType(n);
assert.equal(intType, core.int);
});
});

Expand Down
8 changes: 4 additions & 4 deletions pkg/dev_compiler/test/codegen/expect/misc.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ dart_library.library('misc', null, /* Imports */[
misc.Generic$ = dart.generic(T => {
class Generic extends core.Object {
get type() {
return misc.Generic;
return dart.wrapType(misc.Generic);
}
m() {
return core.print(T);
return core.print(dart.wrapType(T));
}
}
dart.setSignature(Generic, {
Expand Down Expand Up @@ -57,8 +57,8 @@ dart_library.library('misc', null, /* Imports */[
core.print(dart.toString(1.0));
core.print(dart.toString(1.1));
let x = 42;
core.print(dart.equals(x, dart.dynamic));
core.print(dart.equals(x, misc.Generic));
core.print(dart.equals(x, dart.wrapType(dart.dynamic)));
core.print(dart.equals(x, dart.wrapType(misc.Generic)));
core.print(new (misc.Generic$(core.int))().type);
core.print(dart.equals(new misc.Derived(), new misc.Derived()));
new (misc.Generic$(core.int))().m();
Expand Down
52 changes: 52 additions & 0 deletions pkg/dev_compiler/test/codegen/language/type_literal_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// 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";

// TODO(rnystrom): This test has a lot of overlap with some other language
// tests, but those are sort of all over the place so I thought it useful to
// test all of the relevant bits in one place here.

class Foo {}

class Box<T> {
Type get typeArg => T;
}

typedef int Func(bool b);
typedef int GenericFunc<T>(T t);

main() {
// Primitive types.
testType(Object, "Object");
testType(Null, "Null");
testType(bool, "bool");
testType(double, "double");
testType(int, "int");
testType(num, "num");
testType(String, "String");

// Class types.
testType(Foo, "Foo");

// Generic classes.
testType(Box, "Box");
testType(new Box<Foo>().typeArg, "Foo");
testType(new Box<dynamic>().typeArg, "dynamic");
testType(new Box<Box<Foo>>().typeArg, "Box<Foo>");

// Typedef.
testType(Func, "Func");
testType(GenericFunc, "GenericFunc");

// TODO(rnystrom): This should print "GenericFunc<int>", but that isn't
// implemented yet.
testType(new Box<GenericFunc<int>>().typeArg, "GenericFunc");

// Literals are canonicalized.
Expect.identical(Foo, Foo);
Expect.identical(Box, Box);
Expect.identical(new Box<Foo>().typeArg, new Box<Foo>().typeArg);
Expect.identical(Func, Func);
}
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ Object _convertToDart(o) {
var ms = JS('num', '#.getTime()', o);
return new DateTime.fromMillisecondsSinceEpoch(ms);
} else if (o is _DartObject &&
JS('bool', 'dart.jsobject != dart.realRuntimeType(#)', o)) {
JS('bool', 'dart.jsobject != dart.getReifiedType(#)', o)) {
return o._dartObj;
} else {
return _putIfAbsent(_dartProxies, o, _wrapToDart);
Expand Down
2 changes: 1 addition & 1 deletion pkg/dev_compiler/tool/input_sdk/patch/core_patch.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class Object {
}

@patch
Type get runtimeType => JS('Type', 'dart.realRuntimeType(#)', this);
Type get runtimeType => JS('Type', 'dart.objectRuntimeType(#)', this);
}

// Patch for Function implementation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ _ignoreTypeFailure(actual, type) => JS('', '''(() => {
})()''');

strongInstanceOf(obj, type, ignoreFromWhiteList) => JS('', '''(() => {
let actual = $realRuntimeType($obj);
let actual = $getReifiedType($obj);
if ($isSubtype(actual, $type) || actual == $jsobject ||
actual == $int && type == $double) return true;
if ($ignoreFromWhiteList == void 0) return false;
Expand All @@ -217,7 +217,7 @@ instanceOf(obj, type) => JS('', '''(() => {
// TODO(vsm): We can statically detect many cases where this
// check is unnecessary.
if ($isGroundType($type)) return false;
let actual = $realRuntimeType($obj);
let actual = $getReifiedType($obj);
$throwStrongModeError('Strong mode is check failure: ' +
$typeName(actual) + ' does not soundly subtype ' +
$typeName($type));
Expand All @@ -228,7 +228,7 @@ cast(obj, type) => JS('', '''(() => {
// TODO(#296): This is perhaps too eager to throw a StrongModeError?
// TODO(vsm): handle non-nullable types
if ($instanceOfOrNull($obj, $type)) return $obj;
let actual = $realRuntimeType($obj);
let actual = $getReifiedType($obj);
if ($isGroundType($type)) $throwCastError(actual, $type);
if ($_ignoreTypeFailure(actual, $type)) return $obj;
Expand All @@ -243,7 +243,7 @@ asInt(obj) => JS('', '''(() => {
}
if (Math.floor($obj) != $obj) {
// Note: null will also be caught by this check
$throwCastError($realRuntimeType($obj), $int);
$throwCastError($getReifiedType($obj), $int);
}
return $obj;
})()''');
Expand Down Expand Up @@ -390,7 +390,7 @@ final constants = JS('', 'new Map()');
///
@JSExportName('const')
const_(obj) => JS('', '''(() => {
let objectKey = [$realRuntimeType($obj)];
let objectKey = [$getReifiedType($obj)];
// TODO(jmesserly): there's no guarantee in JS that names/symbols are
// returned in the same order.
//
Expand Down
Loading

0 comments on commit 44db042

Please sign in to comment.