dotnet / csharplang

The official repo for the design of the C# programming language
11.53k stars 1.03k forks source link

Champion "Support for == and != on tuple types" (15.7) #190

Open gafter opened 7 years ago

gafter commented 7 years ago

Summary

Support == and != on tuples. For example (x, y) == (1, 2) (which would be equivalent to x == 1 && y == 2).

See also https://github.com/dotnet/roslyn/issues/13155

LDM history:

Started prototype at tuple-equality

DavidArno commented 7 years ago

@jcouv,

I was over-thinking the nested tuples idea. I went down the (a1, (b1, c1)) == (a2, (b2, c2)) route by thinking (b1, c1) == (b2, c2) couldn't work as then they are value tuples, which do not have the == operator. But of course, the compiler can just keep decomposing them, as you say. So scrap the "it'll be confusing" line; that's not true.

DavidArno commented 7 years ago

@jcouv,

I don't expect people to write their own ValueTuple types...

That's a risky assumption to make. I started writing my own version as I wanted ones that were immutable. However I ran into weird compiler errors around ValueTuple<T1,T2> not being assignable to ValueTuple<T1,T2> and the like. Not really had time to investigate whether that was me doing something odd (though I ruled out having any ref to the official tuples) or whether the compiler has some hard-coded dependency on the official version.

It might be prudent therefore, if (when) option 4 is implemented to have the compiler warn on any == and != operators being added to a custom value tuple implementation that the compiler will ignore them.

jnm2 commented 7 years ago

It might be prudent therefore, if (when) option 4 is implemented to have the compiler warn on any == and != operators being added to a custom value tuple implementation that the compiler will ignore them.

As the compiler should. The tuples feature isn't there to serve the ValueTuple implementation; The ValueTuple implementation is there to serve the tuples feature. The central philosophy around tuples is that operations on the tuple should be distributive as operations on each individual value.

DavidArno commented 7 years ago

@jnm2,

Actually, thinking it through, since the team is adverse to introducing breaking changes, they might need to do the reverse. By the time this feature appears in version 7.X, someone may have written a custom value tuple implementation that implements those operators. Overriding them could then be a breaking change. So the compiler might need to respect those operators if present, and use the built-in rules if not.

CyrusNajmabadi commented 7 years ago

I'm with @jnm2 on this. ValueTuple is simply there to serve the language. I'm fine with this being totally at a language level, and ValueTuple not being observed at all once we do this work.

HaloFour commented 7 years ago

@DavidArno

Actually, thinking it through, since the team is adverse to introducing breaking changes, they might need to do the reverse. By the time this feature appears in version 7.X, someone may have written a custom value tuple implementation that implements those operators. Overriding them could then be a breaking change. So the compiler might need to respect those operators if present, and use the built-in rules if not.

Conceptually I agree, however it seems that the team has already made post-C# 7.0 decisions where the compiler may skip the container and work with the elements directly:

https://github.com/dotnet/roslyn/issues/16869

DavidArno commented 7 years ago

@HaloFour,

Ha ha, hoisted by my own petard there! Good call. 🏁

alrz commented 7 years ago

I wonder if this could be implemented on top of shapes rather than hard-coding into the compiler. Not only equality, almost all of tuple operations should be distributed over all elements (if all of them satisfy a certain set of constraints which is representable through generic shapes).

jcouv commented 7 years ago

@DavidArno I'd like to add one clarification (related dotnet/roslyn#16869): both C# and VB languages and compilers now assume that the ValueTuple constructor is side-effect-free, so it can be skipped in some cases. Out of curiosity, would you mind sharing some scenarios that motivate you to implement your own ValueTuple type?

Thanks @HaloFour for making the connection between those two discussions.

DavidArno commented 7 years ago

@jcouv,

I have (well had, see below) a single motivation. I don't like the fact that value tuples are mutable. The natural conclusion (in my head, anyway) was therefore to create my own - immutable - version.

However, I've reconsidered that plan, based on two things:

  1. The compiler will, from 7.1 onwards (or is it in 7?) make assumptions about the behaviour of those tuples,
  2. I read today (from a Roslyn issue you were involved in, I think, but I can't find it now), that from eg .NET framework v4.7, the ValueTuple and new async feature nuget packages will be baked into the framework.

Therefore, I no longer consider it wise to create a custom ValueTuple implementation.

HaloFour commented 7 years ago

@alrz

I wonder if this could be implemented on top of shapes rather than hard-coding into the compiler. Not only equality, almost all of tuple operations should be distributed over all elements (if all of them satisfy a certain set of constraints which is representable through generic shapes).

e.g. using over a tuple full of IDisposables or await over a tuple full of elements for which the compiler can resolve GetAwaiter()?

In those cases it might make sense, but I think it might not always be so obvious how an operation should be distributed, if it should be at all. For the most appropriate behavior it might be necessary for the compiler to special-case the operations in question.

alrz commented 7 years ago

I think it might not always be so obvious how an operation should be distributed, if it should be at all. For the most appropriate behavior it might be necessary for the compiler to special-case the operations in question.

That directly depends on the shape implementation. I'm not sure why "special-casing" would make that more "appropriate". I think this would be the most sensible starting point for designing shapes. Once it's possible to model various operations through shapes there is no reason to special-case such features. IMO it'd be worth it to invest on the general solution rather than special-casing all these into the compiler.

Richiban commented 7 years ago

@alrz

Once it's possible to model various operations through shapes there is no reason to special-case such features

I would normally agree with you, except I do think that it might be appropriate in the case of tuples. Because they should really be such a basic and integral feature I'm all in favour of the compiler 'inlining' the body of the '==' method for tuples, even if the method exists (at least notionally) as the operator '==' on a ValueTuple where T1, T2 etc are members of an Eq typeclass.

alrz commented 7 years ago

The case for inlininig was discussed at length in other threads before and concluded that roslyn is not the place for these optimizations (even though there were some good arguments against it, for instance, https://github.com/dotnet/roslyn/issues/15644). In fact, that was my first concern regarding how shapes are implemented (https://github.com/dotnet/csharplang/issues/164) but as others have said, invocations of shape methods get specialized by the JIT.

alrz commented 6 years ago

How is this feature different from tuple patterns?

(1, 2, 3) is (1, 2, 3)
(1, 2, 3) == (1, 2, 3)

I think it's not, other than that you could compare variables with ==, right?

jcouv commented 6 years ago

@alrz This feature allows comparing two tuple variables (tuple1 == tuple2) as well, whereas positional patterns require constants.

DavidArno commented 6 years ago

@jcouv,

That doesn't sound right to me. Surely positional patterns for tuples will support recursive patterns, not just constants?

tuple is (var x, (1, var y),  3)

@alrz, Surely "I think it's not, other than that you could compare variables with ==, right?" applies to all basic patterns, doesn't it?

x is 1
x == 1
HaloFour commented 6 years ago

@DavidArno

Yes, but the only actual values that you can compare must be themselves constants. So if you're comparing to other variables via pattern matching you'd have to use pattern variables and guards:

public void Foo((int, int) x, (int, int) y) {
    switch (x) {
        case (var a, var b) when a == y.Item1 && b == y.Item2: ...
    }
    // vs
    if (x == y) { ... }
}

I assume that you could also use it as shorthand for comparing equality of multiple items:

public void Foo(int a, int b, int c, int d, int e, int f) {
    if (a == d && b == e && c == f) { ... }
    // vs
    if ((a, b, c) == (d, e, f)) { ... }
}
DavidArno commented 6 years ago

@HaloFour

I absolutely love the idea of:

public void Foo(int a, int b, int c, int d, int e, int f) {
    if ((a, b, c) == (d, e, f)) { ... }
}

Hopefully, @jcouv can work the same magic as he did for the immodestly named Arno Assignment Pattern so that the tuples get optimised away here too.

jcouv commented 6 years ago

@DavidArno (a, b, c) == (d, e, f) will likely require referencing the ValueTuple types for the compilation to succeed, but no ValueTuple reference will be emitted in IL.

jnm2 commented 6 years ago

@jcouv Couldn't the ValueTuple requirement be fixed the same way it was for deconstruction?

jcouv commented 6 years ago

@jnm2 It's not obvious. Still looking into it ;-)