Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal for tuple equality/inequality comparisons #967

Merged
merged 10 commits into from
Mar 20, 2018

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented Oct 4, 2017

Tagging @MadsTorgersen @dotnet/roslyn-compiler for review.

Update: I will update this proposal with input from Neal and Mads:

  • the existence of a conversion from e to a 2-tuple does not affect e == (x, y) (ie. it fails)
  • there is an open question on (x, y) == (1, "") versus (x, y) == (1, null) when there is an == between x and byte. The second one should clearly succeed since the tuple (1, null) has no natural type, we can give it type (byte, string). But should the first one succeed too? (it does in my current prototype, but we'll have to figure out how to word this because the conversion from 1 to byte only works on constants, but the tuple is not a constant). I should test (byte, string) t = ... and (byte x, string y) = ... in those cases too.

Relates to championed issue #190

@jcouv jcouv self-assigned this Oct 4, 2017
@CyrusNajmabadi
Copy link
Member

Conversely it would allow t1 != t2 and evaluate it as temp1.Item1 != temp2.Item1 || temp1.Item2 != temp2.Item2.

Hrmm... maybe it might be better to define it as:

It would allow t1 != t2 and evaluate it as !(t1 == t2).

Thoughts?

@CyrusNajmabadi
Copy link
Member

I bring it up because it seems conceptually simpler, and requires no usage of != and || in the translation.

@HaloFour
Copy link
Contributor

HaloFour commented Oct 6, 2017

@CyrusNajmabadi

That might not technically behave the same way if the types of the elements within the tuple implement custom equality/inequality operators that aren't mirrors of each other for whatever reason.

@VSadov
Copy link
Member

VSadov commented Oct 6, 2017

Why disallowing dynamic elements? Seems odd. I think if I do the transformation by hand, then dynamic equality will be used for those elements. Seems like compiler could do the same.

Copy link
Member

@VSadov VSadov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
I think dynamic elements could/should just work.

@gafter
Copy link
Member

gafter commented Oct 6, 2017

@VSadov Dynamic is "infectious". If it is used for one subexpression, then the result of that subexpression is of type dynamic and the dynamic binder is used for every subexpression above. But the dynamic binder does not (and will not anytime soon) know how to do tuple equality, so it simply would not work.

@orthoxerox
Copy link

@HaloFour @CyrusNajmabadi doubles are a good example of why you cannot replace a != b with !(a == b).

@alrz
Copy link
Member

alrz commented Oct 6, 2017

I still think that it's best to leave this to trait impls: you can only compare two tuples if all the elements are comparable, e.g.

implement<T, U> Eq<T, U> for (T, U) where T : Eq where U : Eq {
    static bool operator ==((T, U) left, (T, U) right)
        => left.Item1 == right.Item1 && left.Item2 == right.Item2;
}

Sure it won't work for all tuple arities, but I think we can get by, just like Rust did:

https://doc.rust-lang.org/std/primitive.tuple.html#trait-implementations

@orthoxerox
Copy link

@alrz this would mean tuple equality would have to be delayed until typeclasses/shapes/traits/concepts are implemented.

@alrz
Copy link
Member

alrz commented Oct 6, 2017

@orthoxerox

Yes, still better than this whole specialized code in the compiler. I didn't find myself to be desperate about this feature as much as I am for ADTs and generators, and still here we are. That's my least concern regarding "when" we can have it. Probably next thing you might want to be specialized is value == (1, 2) which could be a shorthand for value == 1 || value ==2 but that's what OR patterns are all about, right?

@orthoxerox
Copy link

@alrz I would want this to return false unless value actually is a tuple of 1 and 2.

@alrz
Copy link
Member

alrz commented Oct 6, 2017

@orthoxerox

Just a comparison of specializing a syntax vs. a feature that might take a few releases to come, I didn't mean the exact thing.

@gafter
Copy link
Member

gafter commented Oct 6, 2017

@alrz this feature permits comparing a (int, int) to a (long, long). Your type class (not trait) implementation does not.

@alrz
Copy link
Member

alrz commented Oct 6, 2017

@gafter I suppose the compiler can manage implicit conversions for operators as it does today:

var x = implicitlyConvertibleToInteger == 0;

Same works in Rust in a similar manner: numeric promotion is distributed over all tuple elements, so

let x = (1, 2) == (1u64, 2u64);

works as expected - calls the operator == defined in Eq trait, after lhs is promoted to (u64, u64).

@gafter
Copy link
Member

gafter commented Oct 6, 2017

@alrz When comparing an (int, long) to a (long, int), or more complex situations with nested tuples, the usual promotion mechanism doesn't come into play. Or, to put it another way, the current proposal has the same effect but without attempting to define a distributed form of promotion. Note that it would have to promote for more than just numbers; implicit user-defined conversions can be applied at any element.

@alrz
Copy link
Member

alrz commented Oct 6, 2017

@gafter You're right, (int, long) -> (long, int) wouldn't work that way. But, for instance, extension methods on tuples do check permitted conversions in a distributed/recursive manner. So we could do the same when we're calling trait operators, as a generalization of this proposal, retaining the same behavior.

@jcouv jcouv changed the title Spec for tuple equality/inequality comparisons Proposal for tuple equality/inequality comparisons Oct 11, 2017
- `new A(1)`
- `new B(2)`
- `new B(3)`
- `new A(1)`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that supposed to be new B(4) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Corrected

@jcouv jcouv merged commit 351dc30 into dotnet:master Mar 20, 2018
@jcouv jcouv deleted the tuple-equality branch March 20, 2018 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants