-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Existing equality operators should take precedence over compiler lowering for tuple equality #26809
Comments
Kind of late now. |
So is it a bug or not? Is it supposed to be picked over extension equality operators too? It would be kinda really strange if extension operators would work and normal operators would not. It would also be quite strange if some specific extension operators won't work with some specific types. However since current design was tested and reviewed it seems intentional. @jcouv could you please shed some light on this? [Fact]
public void TestCustomOperatorPreferred()
{
var source = @"
namespace System
{
public struct ValueTuple<T1, T2>
{
public T1 Item1;
public T2 Item2;
public ValueTuple(T1 item1, T2 item2)
{
this.Item1 = item1;
this.Item2 = item2;
}
public static bool operator ==(ValueTuple<T1, T2> t1, ValueTuple<T1, T2> t2)
=> throw null;
public static bool operator !=(ValueTuple<T1, T2> t1, ValueTuple<T1, T2> t2)
=> throw null;
public override bool Equals(object o)
=> throw null;
public override int GetHashCode()
=> throw null;
}
}
public class C
{
public static void Main()
{
var t1 = (1, 1);
var t2 = (2, 2);
System.Console.Write(t1 == t2);
System.Console.Write(t1 != t2);
}
}
";
var comp = CreateStandardCompilation(source, options: TestOptions.DebugExe);
comp.VerifyDiagnostics();
// Note: tuple equality picked ahead of custom operator== (small compat break)
CompileAndVerify(comp, expectedOutput: "FalseTrue");
}
[Fact]
void TestTupleEqualityPreferredOverCustomOperator_Nested()
{
string source = @"
public class C
{
public static void Main()
{
System.Console.Write( (1, 2, (3, 4)) == (1, 2, (3, 4)) );
}
}
namespace System
{
public struct ValueTuple<T1, T2>
{
public T1 Item1;
public T2 Item2;
public ValueTuple(T1 item1, T2 item2)
{
this.Item1 = item1;
this.Item2 = item2;
}
public static bool operator ==(ValueTuple<T1, T2> t1, ValueTuple<T1, T2> t2)
=> throw null;
public static bool operator !=(ValueTuple<T1, T2> t1, ValueTuple<T1, T2> t2)
=> throw null;
public override bool Equals(object o)
=> throw null;
public override int GetHashCode()
=> throw null;
}
public struct ValueTuple<T1, T2, T3>
{
public T1 Item1;
public T2 Item2;
public T3 Item3;
public ValueTuple(T1 item1, T2 item2, T3 item3)
{
this.Item1 = item1;
this.Item2 = item2;
this.Item3 = item3;
}
}
}
";
var comp = CreateStandardCompilation(source, options: TestOptions.DebugExe);
comp.VerifyDiagnostics();
// Note: tuple equality picked ahead of custom operator==
CompileAndVerify(comp, expectedOutput: "True");
} |
Sorry for the late reply. I was thrown off by the use of |
Also, yes, the tuple comparison operators come ahead of any extensions, if/when "extension everything" is implemented. |
Currently compiler lowers tuple equality to
t1.Item1 == t2.Item1 && t1.Item2 == t2.Item2 && ...
even when there is an existing and applicable equality operator.While it might not be a big problem by itself as hopefully nobody writes their own
System.ValueTuple
I wonder how extension everyting language feature would interact with this lowering since it would be possible to provide an extension equality operator for theSystem.ValueTuple
type family.Steps to Reproduce:
Compiler the following code with VS 15.6.x:
It compiles without an error since it's allowed to provide your own
System.ValueTuple
type.Now upgrade to VS 15.7 and C# 7.3.
Expected Behavior:
Code still compiles and uses equality operators.
Actual Behavior:
There are multiple errors for missing tuple members.
If you add these members you'll still get different behavior from the previous version since the equality expression will be lowered instead of applying the existing and applicable operator.
The text was updated successfully, but these errors were encountered: