[Analyzer Proposal]: Don't include non-`IEquatable` structs in records

[Youssef1313]

Background and motivation

Using records in C# can be tricky (performance-wise) if the developer isn't aware of some implementation details. The compiler generates the equality implementation using EqualityComparer<T>.Default, which is ObjectEqualityComparer for a struct that is not IEquatable<T>.

Using ObjectEqualityComparer will cause boxing for every equality operation being done.

API Proposal

An analyzer that warns if a record primary constructor has a parameter which is a value type (struct) that doesn't implement IEquatable<T> where T is the same value type.

For example:

using System.Diagnostics.CodeAnalysis;

var bad1 = new BadRecord(default);
var bad2 = new BadRecord(default);

Console.WriteLine(bad1 == bad2); // boxing because `ObjectEqualityComparer` is being used

public struct MyPoint
{
    public int X { get; }
    public int Y { get; }

    public MyPoint(int x, int y)
    {
        X = x;
        Y = y;
    }

    public override bool Equals([NotNullWhen(true)] object? obj)
    {
        return obj is MyPoint point &&
               this == point;
    }

    public static bool operator ==(MyPoint left, MyPoint right)
    {
        return left.X == right.X && left.Y == right.Y;
    }

    public static bool operator !=(MyPoint left, MyPoint right)
    {
        return !(left == right);
    }

    public override int GetHashCode()
    {
        return HashCode.Combine(X, Y);
    }
}

public record struct BadRecord(MyPoint Point); // Produce warning here

API Usage

// Fancy the value
var c = new MyFancyCollection<int>();
c.Fancy(42);

// Getting the values out
foreach (var v in c)
    Console.WriteLine(v);

Alternative Designs

No response

Risks

No response

[EgorBo]

Using ObjectEqualityComparer will cause boxing for every equality operation being done.

I think it's rather an unfortunate RyuJIT's limitation (mainly, https://github.com/dotnet/runtime/issues/9118) and should be fixed in future, so should we introduce temporarily analyzers?

What I guess makes sense to do is to warn if a struct doesn't override Equals at all probably (if it's not already a warning/suggestion).

[dotnet-policy-service[bot]]

Tagging subscribers to this area: @dotnet/area-system-runtime See info in area-owners.md if you want to be subscribed.