Skip to content

Commit

Permalink
Don't wrap type literals that are referenced by prefixes.
Browse files Browse the repository at this point in the history
Also, fix the language test for this to actually do something useful.
I accidentally removed the definition of testType() in the last patch,
so the test did nothing. :(

Fixes #547.

R=vsm@google.com

Review URL: https://codereview.chromium.org/1945643005 .
  • Loading branch information
munificent committed May 4, 2016
1 parent 642d08f commit 3bb9e0a
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 5 deletions.
31 changes: 27 additions & 4 deletions pkg/dev_compiler/lib/src/compiler/code_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2037,10 +2037,7 @@ class CodeGenerator extends GeneralizingAstVisitor

// 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) {
if (!_isInForeignJS && _isTypeLiteral(node)) {
typeName = js.call('dart.wrapType(#)', typeName);
}

Expand Down Expand Up @@ -2088,6 +2085,32 @@ class CodeGenerator extends GeneralizingAstVisitor
return new JS.Identifier(name);
}

/// Returns `true` if the type name referred to by [node] is used in a
/// position where it should evaluate as a type literal -- an object of type
/// Type.
bool _isTypeLiteral(SimpleIdentifier node) {
var parent = node.parent;

// Static member call.
if (parent is MethodInvocation || parent is PropertyAccess) return false;

// An expression like "a.b".
if (parent is PrefixedIdentifier) {
// In "a.b", "b" may be a type literal, but "a", is not.
if (node != parent.identifier) return false;

// If the prefix expression is itself used as an invocation, like
// "a.b.c", then "b" is not a type literal.
var grand = parent.parent;
if (grand is MethodInvocation || grand is PropertyAccess) return false;

return true;
}

// In any other context, it's a type literal.
return true;
}

JS.Identifier _emitParameter(ParameterElement element,
{bool declaration: false}) {
// initializing formal parameter, e.g. `Point(this._x)`
Expand Down
25 changes: 24 additions & 1 deletion pkg/dev_compiler/test/codegen/language/type_literal_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@

import "package:expect/expect.dart";

import 'type_literal_test.dart' as prefix;

// 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 Foo {
static var property;
static method() => "result";
}

class Box<T> {
Type get typeArg => T;
Expand Down Expand Up @@ -49,4 +54,22 @@ main() {
Expect.identical(Box, Box);
Expect.identical(new Box<Foo>().typeArg, new Box<Foo>().typeArg);
Expect.identical(Func, Func);

// Static member uses are not type literals.
Foo.property = "value";
Expect.equals("value", Foo.property);
Expect.equals("result", Foo.method());

// Prefixed types are type literals.
testType(prefix.Foo, "Foo");

// Prefix member uses are not.
prefix.Foo.property = "value2";
Expect.equals("value2", prefix.Foo.property);
Expect.equals("result", prefix.Foo.method());
}

void testType(Type type, String string) {
Expect.equals(string, type.toString());
Expect.isTrue(type is Type);
}

0 comments on commit 3bb9e0a

Please sign in to comment.