Implementations IEquatable<T> for custom structs.

[sq735]

Motivation:

Best practices recommend implementing an "IEquatable<T>" interface for custom structs. This improves the performance of comparison operations and eliminates boxing.

[nblumhardt]

Thanks for the PR! Is there anywhere currently in the code path that we'd expect this to speed up?

Also, especially if so, should we first consider making these types into C#10 record structs?

(I haven't dug into this in quite some time, so please excuse some possibly obvious questions :-))

[sq735]

Thanks for the PR! Is there anywhere currently in the code path that we'd expect this to speed up?

It's hard to say since the equivalence methods can be called implicitly by the framework for example, when working with "LINQ" or some collections. For this reason, it is always better to implement them explicitly so you don't have to think about where the default implementation can become a bottleneck.

Also, especially if so, should we first consider making these types into C#10 record structs?

Yes, you are right. Record structs are preferred because the compiler generates the equivalence methods (and some other methods) for us. In PR I didn't make the structures as record structures because that seemed like a more fundamental change to me. If you'll allow me, I can make them as record structs.

(I haven't dug into this in quite some time, so please excuse some possibly obvious questions :-))

You can see what happens behind the scenes when we create record structs here. In short, by default the compiler implements the operator ==, != methods GetHashCode, overrides Equals, implements IEquatable<T>.Equals and ToString. If necessary, we can intervene in this process and make our own adjustments if the implementation does not suit us.

[nblumhardt]

Hi! Thanks for your reply. Giving this some thought, it would probably be a bug if we were using struct equality implicitly anywhere in this codebase, so adding support for it may be confusing (we don't generally want to use it, so the impls there would be misleading). Let me know if this matches your impressions.

Best regards, Nick

[sq735]

I don't think an explicit implementation would be misleading. But if you are of a different opinion, then the record struct are your choice. Since they do not require an explicit implementation of equivalence and eliminate a number of problems inherent in classical structures.

[nblumhardt]

Thanks, Pavel - giving it some more thought 👍

[sq735]

Nicholas, you can see some performance comparison for structs and record struct here.

[nblumhardt]

Thanks again for sending this. I've run back over everything and decided not to merge this one in order to keep the codebase lean. Although there's some hypothetical benefit, these structs are not intended to be used as comparable values and in current usage the added methods won't be used.

In the case of Position, which is a reasonable candidate for use as a comparable value, the different types of comparisons are nuanced, and in the cases we care about, only Absolute is compared (since the counted line/column info has no bearing on the case of identifying a span within a string).

If comparison using one of these types does show up in a scenario somewhere we can definitely revisit. Thanks again @sq735!