fix #27040, promote the upper bound of a type parameter

fix #27040, promote the upper bound of a type parameter

also fixes #27210, by cleaning up TypeParameterMember equality TODO.

R=leafp@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/4213059b74f217372d11ea9327afd32f6a28f4c2

[Jennifer Messerly, 2016-09-29 07:31:33]

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

[Jennifer Messerly, 2016-09-29 07:31:33]

This builds on earlier CL's, in particular
https://codereview.chromium.org/2375103004/

[Leaf, 2016-09-30 05:00:04]

lgtm

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

https://codereview.chromium.org/2375373002/diff/1/pkg/analyzer/test/src/task/...
pkg/analyzer/test/src/task/strong/checker_test.dart:3717: void h/*<T extends
Clonable<T>>*/(/*=T*/ object) {
Test that object is usable as a SubClonable<T> as well as a T?

[Jennifer Messerly, 2016-09-30 16:58:03]

Thanks!

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

https://codereview.chromium.org/2375373002/diff/1/pkg/analyzer/test/src/task/...
pkg/analyzer/test/src/task/strong/checker_test.dart:3717: void h/*<T extends
Clonable<T>>*/(/*=T*/ object) {
On 2016/09/30 05:00:04, Leaf wrote:
> Test that object is usable as a SubClonable<T> as well as a T?

I think `object.m` is supposed to illustrate that. Perhaps would be better not
all on the same line though and with comments.

[Leaf, 2016-09-30 17:00:30]

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

https://codereview.chromium.org/2375373002/diff/1/pkg/analyzer/test/src/task/...
pkg/analyzer/test/src/task/strong/checker_test.dart:3717: void h/*<T extends
Clonable<T>>*/(/*=T*/ object) {
On 2016/09/30 16:58:03, John Messerly wrote:
> On 2016/09/30 05:00:04, Leaf wrote:
> > Test that object is usable as a SubClonable<T> as well as a T?
> 
> I think `object.m` is supposed to illustrate that. Perhaps would be better not
> all on the same line though and with comments.

Sort of.  object.m tests that SubClonable methods can be called on it.  Just to
be paranoid I thought it might be useful to check that assignability actually
works as well, since that follows different code paths

[Jennifer Messerly, 2016-10-20 23:25:00]

On 2016/09/30 17:00:30, Leaf wrote:
>
https://codereview.chromium.org/2375373002/diff/1/pkg/analyzer/test/src/task/...
> File pkg/analyzer/test/src/task/strong/checker_test.dart (right):
> 
>
https://codereview.chromium.org/2375373002/diff/1/pkg/analyzer/test/src/task/...
> pkg/analyzer/test/src/task/strong/checker_test.dart:3717: void h/*<T extends
> Clonable<T>>*/(/*=T*/ object) {
> On 2016/09/30 16:58:03, John Messerly wrote:
> > On 2016/09/30 05:00:04, Leaf wrote:
> > > Test that object is usable as a SubClonable<T> as well as a T?
> > 
> > I think `object.m` is supposed to illustrate that. Perhaps would be better
not
> > all on the same line though and with comments.
> 
> Sort of.  object.m tests that SubClonable methods can be called on it.  Just
to
> be paranoid I thought it might be useful to check that assignability actually
> works as well, since that follows different code paths

I'll add some of these test cases (including the one you mentioned here:
https://codereview.chromium.org/2375103004/) and then take another shot at this
one.

[Jennifer Messerly, 2016-10-24 17:56:16]

Description was changed from

==========
fix #27040, promote the upper bound of a type parameter
==========

to

==========
fix #27040, promote the upper bound of a type parameter

also fixes #27210, by cleaning up TypeParameterMember equality TODO.
==========

[Jennifer Messerly, 2016-10-24 23:31:28]

Hi Leaf, PTAL.

There's still some pre-existing weirdness in Analyzer I'm not sure what to make
of...

TypeParameterTypeImpl.== doesn't consider the bound at all. This seems
problematic, as bound can be substituted, even before this CL (that's what
TypeParameterMember is for).

The reason it doesn't handle the bound, I think, is recursive bounds. It's
tricky to implement that with operator ==.

So overall I'm not sure whether this fix is worth it. The usability improvement
is nice but it's built on a creaky thing in Analyzer. That said, even without
the CL there's still some problematic stuff for type parameters. Advice
appreciated on how to proceed.

(it's funny this bug trips on all the problem areas in Analyzer's Types,
including:
* lazy substitution
* operator == meaning
* members
* that types need backing elements)

[Leaf, 2016-11-01 17:54:52]

lgtm.

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

https://codereview.chromium.org/2375373002/diff/40001/pkg/analyzer/test/src/t...
pkg/analyzer/test/src/task/strong/checker_test.dart:3769: // type, and the way
it is implemented, this leads back to `T extends num`.
This is kind of a bug. Maybe file an issue and add a failing test?

[Jennifer Messerly, 2016-11-02 01:15:07]

Description was changed from

==========
fix #27040, promote the upper bound of a type parameter

also fixes #27210, by cleaning up TypeParameterMember equality TODO.
==========

to

==========
fix #27040, promote the upper bound of a type parameter

also fixes #27210, by cleaning up TypeParameterMember equality TODO.

R=leafp@google.com

Committed:
https://github.com/dart-lang/dev_compiler/commit/4213059b74f217372d11ea9327af...
==========

[Jennifer Messerly, 2016-11-02 01:15:08]

Committed patchset #3 (id:40001) manually as
4213059b74f217372d11ea9327afd32f6a28f4c2 (presubmit successful).

[Jennifer Messerly, 2016-11-02 01:17:08]

thanks! landed

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

https://codereview.chromium.org/2375373002/diff/40001/pkg/analyzer/test/src/t...
pkg/analyzer/test/src/task/strong/checker_test.dart:3769: // type, and the way
it is implemented, this leads back to `T extends num`.
On 2016/11/01 17:54:52, Leaf wrote:
> This is kind of a bug. Maybe file an issue and add a failing test?

done: https://github.com/dart-lang/sdk/issues/27725