fix #27210, remove TypeParameterMember equality

fix #27210, remove TypeParameterMember equality

[Jennifer Messerly, 2016-09-29 07:08:34]

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

[Jennifer Messerly, 2016-09-29 07:08:34]

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/dart/eleme...
File pkg/analyzer/lib/dart/element/type.dart (right):

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/dart/eleme...
pkg/analyzer/lib/dart/element/type.dart:708: DartType get bound;
trying to make TypeParameterType instances usable on their own, without needing
to consult the element API.

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/type_system.dart (left):

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/type_system.dart:931: if (t1 == t2) {
this builds on https://codereview.chromium.org/2381773002/

[Leaf, 2016-09-30 05:08:23]

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/type_system.dart (left):

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/type_system.dart:931: if (t1 == t2) {
On 2016/09/29 07:08:34, John Messerly wrote:
> this builds on https://codereview.chromium.org/2381773002/

Wait, that one says it builds on this one.  Are you *trying* to overflow my
stack?  ;)

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/type_system.dart (right):

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/type_system.dart:933: isSubtypeOf(t1.bound,
t2.bound)) {
I'm not sure I understand what this subtype check is accomplishing.  It feels
either unnecessary or wrong to me, but I think that probably means that I'm
missing something.

[Jennifer Messerly, 2016-09-30 23:23:12]

Thanks! PTAL.

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/type_system.dart (left):

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/type_system.dart:931: if (t1 == t2) {
On 2016/09/30 05:08:22, Leaf wrote:
> On 2016/09/29 07:08:34, John Messerly wrote:
> > this builds on https://codereview.chromium.org/2381773002/
> 
> Wait, that one says it builds on this one.  Are you *trying* to overflow my
> stack?  ;)

oh does it? I didn't notice that. In any case it already landed:
https://github.com/dart-lang/sdk/commit/d614c8dcd40fc1b11200928cad0f3b65f9983bd8

sometimes I worry about linking CL's too, because the emails that get sent make
it very hard to see which link was the comment that I wrote, vs which link is
the link back to the CL that contained the comment.

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/type_system.dart (right):

https://codereview.chromium.org/2375103004/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/type_system.dart:933: isSubtypeOf(t1.bound,
t2.bound)) {
On 2016/09/30 05:08:22, Leaf wrote:
> I'm not sure I understand what this subtype check is accomplishing.  It feels
> either unnecessary or wrong to me, but I think that probably means that I'm
> missing something. 

Oh, right. It should be an equality check on the bound. DOH.

PTAL, I've attempted to fix this in the right place ... TypeParameterType
equality.

[Jennifer Messerly, 2016-10-01 00:02:34]

@Leaf, in case it helps, the idea to refine the bound was from your suggestion
here: https://github.com/dart-lang/sdk/issues/26965#issuecomment-237610598

I'm happy to merge this with the other CL too if that makes it easier to see how
they're related.

[Jennifer Messerly, 2016-10-18 01:16:18]

On 2016/10/01 00:02:34, Jennifer Messerly wrote:
> @Leaf, in case it helps, the idea to refine the bound was from your suggestion
> here: https://github.com/dart-lang/sdk/issues/26965#issuecomment-237610598
> 
> I'm happy to merge this with the other CL too if that makes it easier to see
how
> they're related.

Hey Leaf, wondering where we're at with this CL
This other CL depends on this one: https://codereview.chromium.org/2375373002

Should I merge them? Any advice on what to do?

[Leaf, 2016-10-18 20:39:58]

On 2016/10/18 01:16:18, Jennifer Messerly wrote:
> On 2016/10/01 00:02:34, Jennifer Messerly wrote:
> > @Leaf, in case it helps, the idea to refine the bound was from your
suggestion
> > here: https://github.com/dart-lang/sdk/issues/26965#issuecomment-237610598
> > 
> > I'm happy to merge this with the other CL too if that makes it easier to see
> how
> > they're related.
> 
> Hey Leaf, wondering where we're at with this CL
> This other CL depends on this one: https://codereview.chromium.org/2375373002
> 
> Should I merge them? Any advice on what to do?

Right sorry.  Yes, I think merging them would be helpful.  Also, I was worried
about the following kinds of cases:

void f<T <: num>(T x, T y) {
  var z = x;
  var f = () => x;
  f = () => y;
  if (x is int) {
    z.isEven; // Should be an error, make sure that promotion doesn't carry
over?
    x = z; // Should be a downcast, make sure that it is?
    f().isEven; // Should be an error, make sure that it is
    var g = () => x;
    g = f; // Should be an error, make sure that it is
    g().isEven; // Should work
    x = g(); // Probably should work?
    int r = x; // should work
  }
}

Can you add tests to make sure that all of this works as intended?

[Jennifer Messerly, 2016-10-20 23:24:12]

On 2016/10/18 20:39:58, Leaf wrote:
> On 2016/10/18 01:16:18, Jennifer Messerly wrote:
> > On 2016/10/01 00:02:34, Jennifer Messerly wrote:
> > > @Leaf, in case it helps, the idea to refine the bound was from your
> suggestion
> > > here: https://github.com/dart-lang/sdk/issues/26965#issuecomment-237610598
> > > 
> > > I'm happy to merge this with the other CL too if that makes it easier to
see
> > how
> > > they're related.
> > 
> > Hey Leaf, wondering where we're at with this CL
> > This other CL depends on this one:
https://codereview.chromium.org/2375373002
> > 
> > Should I merge them? Any advice on what to do?
> 
> Right sorry.  Yes, I think merging them would be helpful.  Also, I was worried
> about the following kinds of cases:
> 
> void f<T <: num>(T x, T y) {
>   var z = x;
>   var f = () => x;
>   f = () => y;
>   if (x is int) {
>     z.isEven; // Should be an error, make sure that promotion doesn't carry
> over?
>     x = z; // Should be a downcast, make sure that it is?
>     f().isEven; // Should be an error, make sure that it is
>     var g = () => x;
>     g = f; // Should be an error, make sure that it is
>     g().isEven; // Should work
>     x = g(); // Probably should work?
>     int r = x; // should work
>   }
> }
> 
> Can you add tests to make sure that all of this works as intended?

Excellent I'll merge this with the other one. Closing this one out. :)