fix #26393, treat tear offs as definite functions

fix #26393, treat tear offs as definite functions

R=vsm@google.com

Committed: https://github.com/dart-lang/sdk/commit/ed416bb6cb3a7f4cb1fdf65ec589925ce3662ae6

[Jennifer Messerly, 2016-08-05 20:31:12]

jmesserly@google.com changed reviewers:
+ leafp@google.com, vsm@google.com

[vsm, 2016-08-05 21:41:32]

https://codereview.chromium.org/2212233003/diff/20001/pkg/analyzer/test/src/t...
File pkg/analyzer/test/src/task/strong/inferred_type_test.dart (right):

https://codereview.chromium.org/2212233003/diff/20001/pkg/analyzer/test/src/t...
pkg/analyzer/test/src/task/strong/inferred_type_test.dart:1761:
takeOON(/*error:STATIC_TYPE_ERROR,error:ARGUMENT_TYPE_NOT_ASSIGNABLE*/new
C().m);
Does the comment above no longer hold?

https://codereview.chromium.org/2212233003/diff/20001/pkg/analyzer/test/src/t...
pkg/analyzer/test/src/task/strong/inferred_type_test.dart:1766:
takeOOI(/*error:STATIC_TYPE_ERROR,error:ARGUMENT_TYPE_NOT_ASSIGNABLE*/new
C().m);
Ditto

[Jennifer Messerly, 2016-08-05 21:49:43]

https://codereview.chromium.org/2212233003/diff/20001/pkg/analyzer/test/src/t...
File pkg/analyzer/test/src/task/strong/inferred_type_test.dart (right):

https://codereview.chromium.org/2212233003/diff/20001/pkg/analyzer/test/src/t...
pkg/analyzer/test/src/task/strong/inferred_type_test.dart:1761:
takeOON(/*error:STATIC_TYPE_ERROR,error:ARGUMENT_TYPE_NOT_ASSIGNABLE*/new
C().m);
On 2016/08/05 21:41:32, vsm wrote:
> Does the comment above no longer hold?  

oops. Yeah I meant to remove that.

[vsm, 2016-08-05 21:57:20]

On 2016/08/05 21:49:43, John Messerly wrote:
>
https://codereview.chromium.org/2212233003/diff/20001/pkg/analyzer/test/src/t...
> File pkg/analyzer/test/src/task/strong/inferred_type_test.dart (right):
> 
>
https://codereview.chromium.org/2212233003/diff/20001/pkg/analyzer/test/src/t...
> pkg/analyzer/test/src/task/strong/inferred_type_test.dart:1761:
> takeOON(/*error:STATIC_TYPE_ERROR,error:ARGUMENT_TYPE_NOT_ASSIGNABLE*/new
> C().m);
> On 2016/08/05 21:41:32, vsm wrote:
> > Does the comment above no longer hold?  
> 
> oops. Yeah I meant to remove that.

An alternative is to modify _recordImplicitCast and allow the cast here
(assuming no --no-implicit-cast).  I think that code assumed that
isKnownFunction implies "has exact type / cast will definitely fail".  But
that's not true with instance methods.

[Leaf, 2016-08-05 23:33:05]

https://codereview.chromium.org/2212233003/diff/40001/pkg/analyzer/test/src/t...
File pkg/analyzer/test/src/task/strong/checker_test.dart (right):

https://codereview.chromium.org/2212233003/diff/40001/pkg/analyzer/test/src/t...
pkg/analyzer/test/src/task/strong/checker_test.dart:1409: f =
/*error:STATIC_TYPE_ERROR*/c.top;
This doesn't feel right.  I think this is the "stupid cast" logic coming into
play?  But this cast could succeed: `c` could contain a subclass of C which
overrides top with the right type (since it is a subtype).  Assuming this is
right about where this comes from, then I think the cast logic that inserts this
error shouldn't do so for methods (since they can be overridden).

[vsm, 2016-08-05 23:40:11]

On 2016/08/05 23:33:05, Leaf wrote:
>
https://codereview.chromium.org/2212233003/diff/40001/pkg/analyzer/test/src/t...
> File pkg/analyzer/test/src/task/strong/checker_test.dart (right):
> 
>
https://codereview.chromium.org/2212233003/diff/40001/pkg/analyzer/test/src/t...
> pkg/analyzer/test/src/task/strong/checker_test.dart:1409: f =
> /*error:STATIC_TYPE_ERROR*/c.top;
> This doesn't feel right.  I think this is the "stupid cast" logic coming into
> play?  But this cast could succeed: `c` could contain a subclass of C which
> overrides top with the right type (since it is a subtype).  Assuming this is
> right about where this comes from, then I think the cast logic that inserts
this
> error shouldn't do so for methods (since they can be overridden).

I believe the code (in _recordImplicitCast) assumes that isKnownFunction => type
is exact.  That no longer holds.

When the type is exact, we give a static error instead of injecting a down cast.

[Jennifer Messerly, 2016-08-08 15:39:11]

Ahhh, yeah that makes sense, we were conflating "exact type" and "strict arrow",
because both are true for static methods. I'll revert the fix here and try and
do it properly

[Jennifer Messerly, 2016-08-08 19:06:27]

Fixed for reals this time :D. PTAL at the new version.

[vsm, 2016-08-08 20:07:14]

lgtm

https://codereview.chromium.org/2212233003/diff/60001/pkg/analyzer/test/src/t...
File pkg/analyzer/test/src/task/strong/checker_test.dart (right):

https://codereview.chromium.org/2212233003/diff/60001/pkg/analyzer/test/src/t...
pkg/analyzer/test/src/task/strong/checker_test.dart:2679:
/*error:INVALID_METHOD_OVERRIDE*/void f(int x) {}
Ah - looks like we're already testing that F.f has a strict type wrt override
checking.  :-)

[Jennifer Messerly, 2016-08-08 21:21:13]

Thanks!

https://codereview.chromium.org/2212233003/diff/60001/pkg/analyzer/test/src/t...
File pkg/analyzer/test/src/task/strong/checker_test.dart (right):

https://codereview.chromium.org/2212233003/diff/60001/pkg/analyzer/test/src/t...
pkg/analyzer/test/src/task/strong/checker_test.dart:2679:
/*error:INVALID_METHOD_OVERRIDE*/void f(int x) {}
On 2016/08/08 20:07:14, vsm wrote:
> Ah - looks like we're already testing that F.f has a strict type wrt override
> checking.  :-)

oh! yes I should've mentioned that, I also did a manual test and confirmed it
was handled correctly for overrides.

[Jennifer Messerly, 2016-08-08 21:29:23]

Description was changed from

==========
fix #26393, treat tear offs as definite functions
==========

to

==========
fix #26393, treat tear offs as definite functions

R=vsm@google.com

Committed:
https://github.com/dart-lang/sdk/commit/ed416bb6cb3a7f4cb1fdf65ec589925ce3662ae6
==========

[Jennifer Messerly, 2016-08-08 21:29:25]

Committed patchset #4 (id:60001) manually as
ed416bb6cb3a7f4cb1fdf65ec589925ce3662ae6 (presubmit successful).