Blacklist types from core library, issue 969

Blacklist types from core library, issue 969
http://code.google.com/p/dart/issues/detail?id=969

R=zundel@google.com
BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=3102

[zundel, 2012-01-06 18:17:20]

http://codereview.chromium.org/9124006/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/resolver/Elements.java (right):

http://codereview.chromium.org/9124006/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/resolver/Elements.java:503: public static
boolean isTypeNode(DartTypeNode typeNode, List<String> names) {
Wouldn't 'names' be better modeled as a Set?  That way you can have constant
time lookup.

http://codereview.chromium.org/9124006/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/resolver/SupertypeResolver.java
(right):

http://codereview.chromium.org/9124006/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/resolver/SupertypeResolver.java:29:
private static final List<String> BLACK_LISTED_TYPES = ImmutableList.of(
With such a small collection of elements, it probably doesn't mater, but as
stated earlier, you could use a set here, initialized with ImmutableSet.of().

http://codereview.chromium.org/9124006/diff/1/compiler/javatests/com/google/d...
File
compiler/javatests/com/google/dart/compiler/resolver/ClassExtendsInterfaceNegativeTest.dart
(left):

http://codereview.chromium.org/9124006/diff/1/compiler/javatests/com/google/d...
compiler/javatests/com/google/dart/compiler/resolver/ClassExtendsInterfaceNegativeTest.dart:9:
class Base extends I {
Wait, this should be an error.  Judging from the JUnit tests, you probably
deleted this test by mistake?

Section 7.8: It is a compile-time error if the extends clause of a class C
includes a type expression that does not denot a class available in the lexical
scope of C.


But it WOULD be legal for 

class Base { }
interface I extends Base {} 

because a class always defines an implicit interface of the same name. (section
7)

http://codereview.chromium.org/9124006/diff/1/compiler/javatests/com/google/d...
File compiler/javatests/com/google/dart/compiler/resolver/ResolverTest.java
(left):

http://codereview.chromium.org/9124006/diff/1/compiler/javatests/com/google/d...
compiler/javatests/com/google/dart/compiler/resolver/ResolverTest.java:1070:
public void test_explicitDynamicTypeArgument() throws Exception {
did you mean to delete this test?  Seems like it is unrelated to this change and
should be working.

[scheglov, 2012-01-09 15:12:28]

http://codereview.chromium.org/9124006/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/resolver/Elements.java (right):

http://codereview.chromium.org/9124006/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/resolver/Elements.java:503: public static
boolean isTypeNode(DartTypeNode typeNode, List<String> names) {
On 2012/01/06 18:17:20, zundel wrote:
> Wouldn't 'names' be better modeled as a Set?  That way you can have constant
> time lookup.

Done.

http://codereview.chromium.org/9124006/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/resolver/SupertypeResolver.java
(right):

http://codereview.chromium.org/9124006/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/resolver/SupertypeResolver.java:29:
private static final List<String> BLACK_LISTED_TYPES = ImmutableList.of(
On 2012/01/06 18:17:20, zundel wrote:
> With such a small collection of elements, it probably doesn't mater, but as
> stated earlier, you could use a set here, initialized with ImmutableSet.of().

Done.

http://codereview.chromium.org/9124006/diff/1/compiler/javatests/com/google/d...
File
compiler/javatests/com/google/dart/compiler/resolver/ClassExtendsInterfaceNegativeTest.dart
(left):

http://codereview.chromium.org/9124006/diff/1/compiler/javatests/com/google/d...
compiler/javatests/com/google/dart/compiler/resolver/ClassExtendsInterfaceNegativeTest.dart:9:
class Base extends I {
On 2012/01/06 18:17:20, zundel wrote:
> Wait, this should be an error.  Judging from the JUnit tests, you probably
> deleted this test by mistake?
> 
> Section 7.8: It is a compile-time error if the extends clause of a class C
> includes a type expression that does not denot a class available in the
lexical
> scope of C.
> 
> 
> But it WOULD be legal for 
> 
> class Base { }
> interface I extends Base {} 
> 
> because a class always defines an implicit interface of the same name.
(section
> 7)

Moved into NegativeResolverTest.test_classExtendsInterface()

Plus added test_classImplementsClass() and test_interfaceExtendsClass().

http://codereview.chromium.org/9124006/diff/1/compiler/javatests/com/google/d...
File compiler/javatests/com/google/dart/compiler/resolver/ResolverTest.java
(left):

http://codereview.chromium.org/9124006/diff/1/compiler/javatests/com/google/d...
compiler/javatests/com/google/dart/compiler/resolver/ResolverTest.java:1070:
public void test_explicitDynamicTypeArgument() throws Exception {
On 2012/01/06 18:17:20, zundel wrote:
> did you mean to delete this test?  Seems like it is unrelated to this change
and
> should be working.

Moved into NegativeResolverTest for better errors validation.

Dynamic tests are not in NegativeResolverTest.test_blackListed_Dynamic().

[zundel, 2012-01-09 16:07:48]

lgtm

http://codereview.chromium.org/9124006/diff/1/compiler/javatests/com/google/d...
File
compiler/javatests/com/google/dart/compiler/resolver/ClassExtendsInterfaceNegativeTest.dart
(left):

http://codereview.chromium.org/9124006/diff/1/compiler/javatests/com/google/d...
compiler/javatests/com/google/dart/compiler/resolver/ClassExtendsInterfaceNegativeTest.dart:9:
class Base extends I {
On 2012/01/09 15:12:28, scheglov wrote:
> On 2012/01/06 18:17:20, zundel wrote:
> > Wait, this should be an error.  Judging from the JUnit tests, you probably
> > deleted this test by mistake?
> > 
> > Section 7.8: It is a compile-time error if the extends clause of a class C
> > includes a type expression that does not denot a class available in the
> lexical
> > scope of C.
> > 
> > 
> > But it WOULD be legal for 
> > 
> > class Base { }
> > interface I extends Base {} 
> > 
> > because a class always defines an implicit interface of the same name.
> (section
> > 7)
> 
> Moved into NegativeResolverTest.test_classExtendsInterface()
> 
> Plus added test_classImplementsClass() and test_interfaceExtendsClass().

Sorry, I mistook this for a shared test, but I now see it is private to dartc.