Fix typo in native_helper and add dartc test.

Fix typo in native_helper and add dartc test.

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

[zundel, 2012-03-30 00:25:00]

https://chromiumcodereview.appspot.com/9955001/diff/1/tests/language/src/Gett...
File tests/language/src/GettersSettersTypeTest.dart (right):

https://chromiumcodereview.appspot.com/9955001/diff/1/tests/language/src/Gett...
tests/language/src/GettersSettersTypeTest.dart:10: get foo() => bar;
First of all, I agree something is going wrong in dartc, but I think there is an
issue with the test:

I don't understand why this is a warning?  top level field bar is an int so the
return type should be OK.

If you are just running case 1 you don't have a conflict
with String type used in the setter because it is commented out.  Its type would
be considered dynamic and dynamic is assignable to int, so the 2 types are valid
for getter/setter on the same field.

If you run case 2 I think it should also run without warnings.  But the fact
that it is passing does expose something.  If the setter has a type, but the
getter is Dynamic, BUT, it assumes the type of the setter for the getter, which
is wrong.

I think the cases you want to test are:

int  /// 01: static type warning 
int  /// 02: ... should run without warning
get foo() {   ... }

void set foo (
  String  /// 01:continued
  String /// 03: should run without warning
  str) { ... }

but I'm not sure how to express cases 2 & 3 with the test framework we've got.

There are still bugs in dartc, though. If you fix up test case 1 to look like
the above, it reports static type warnings on the assignments below, not the
getter and setter definitions where they belong.  I fixed a similar issue in
dartc recently, but it apparently does not work with top level fields.

https://chromiumcodereview.appspot.com/9955001/diff/1/tests/language/src/Gett...
tests/language/src/GettersSettersTypeTest.dart:15: 
FYI, A second test case where getters and setters are in classes works as
expected in dartc:

class A {
  int  
    get foo() => bar;

  void set foo( String
             str) { bar = str.length; }
}

[zundel, 2012-03-30 02:53:51]

https://chromiumcodereview.appspot.com/9955001/diff/1/tests/language/src/Gett...
File tests/language/src/GettersSettersTypeTest.dart (right):

https://chromiumcodereview.appspot.com/9955001/diff/1/tests/language/src/Gett...
tests/language/src/GettersSettersTypeTest.dart:10: get foo() => bar;
On 2012/03/30 00:25:01, zundel wrote:
> First of all, I agree something is going wrong in dartc, but I think there is
an
> issue with the test:
> 
> I don't understand why this is a warning?  top level field bar is an int so
the
> return type should be OK.
> 
> If you are just running case 1 you don't have a conflict
> with String type used in the setter because it is commented out.  Its type
would
> be considered dynamic and dynamic is assignable to int, so the 2 types are
valid
> for getter/setter on the same field.
> 
> If you run case 2 I think it should also run without warnings.  But the fact
> that it is passing does expose something.  If the setter has a type, but the
> getter is Dynamic, BUT, it assumes the type of the setter for the getter,
which
> is wrong.
> 
> I think the cases you want to test are:
> 
> int  /// 01: static type warning 
> int  /// 02: ... should run without warning
> get foo() {   ... }
> 
> void set foo (
>   String  /// 01:continued
>   String /// 03: should run without warning
>   str) { ... }
> 
> but I'm not sure how to express cases 2 & 3 with the test framework we've got.
> 
> There are still bugs in dartc, though. If you fix up test case 1 to look like
> the above, it reports static type warnings on the assignments below, not the
> getter and setter definitions where they belong.  I fixed a similar issue in
> dartc recently, but it apparently does not work with top level fields.
> 
Actually, if you want to write 2 extra (non multi-test) cases, you can use
@static-clean to make sure that cases 2 and 3 outlined above run without errors.

[floitsch, 2012-03-30 05:36:10]

https://chromiumcodereview.appspot.com/9955001/diff/1/tests/language/src/Gett...
File tests/language/src/GettersSettersTypeTest.dart (right):

https://chromiumcodereview.appspot.com/9955001/diff/1/tests/language/src/Gett...
tests/language/src/GettersSettersTypeTest.dart:10: get foo() => bar;
On 2012/03/30 02:53:51, zundel wrote:
> On 2012/03/30 00:25:01, zundel wrote:
> > First of all, I agree something is going wrong in dartc, but I think there
is
> an
> > issue with the test:
> > 
> > I don't understand why this is a warning?  top level field bar is an int so
> the
> > return type should be OK.
> > 
> > If you are just running case 1 you don't have a conflict
> > with String type used in the setter because it is commented out.  Its type
> would
> > be considered dynamic and dynamic is assignable to int, so the 2 types are
> valid
> > for getter/setter on the same field.
> > 
> > If you run case 2 I think it should also run without warnings.  But the fact
> > that it is passing does expose something.  If the setter has a type, but the
> > getter is Dynamic, BUT, it assumes the type of the setter for the getter,
> which
> > is wrong.
> > 
> > I think the cases you want to test are:
> > 
> > int  /// 01: static type warning 
> > int  /// 02: ... should run without warning
> > get foo() {   ... }
> > 
> > void set foo (
> >   String  /// 01:continued
> >   String /// 03: should run without warning
> >   str) { ... }
> > 
> > but I'm not sure how to express cases 2 & 3 with the test framework we've
got.
> > 
> > There are still bugs in dartc, though. If you fix up test case 1 to look
like
> > the above, it reports static type warnings on the assignments below, not the
> > getter and setter definitions where they belong.  I fixed a similar issue in
> > dartc recently, but it apparently does not work with top level fields.
> > 
> Actually, if you want to write 2 extra (non multi-test) cases, you can use
> @static-clean to make sure that cases 2 and 3 outlined above run without
errors.

Done.

[ngeoffray, 2012-03-30 08:35:28]

LGTM

[zundel, 2012-03-30 12:19:40]

lgtm2