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

Consider having enums auto-implement IEquatable<T> #2349

Open
GrabYourPitchforks opened this issue Mar 15, 2019 · 17 comments
Open

Consider having enums auto-implement IEquatable<T> #2349

GrabYourPitchforks opened this issue Mar 15, 2019 · 17 comments

Comments

@GrabYourPitchforks
Copy link
Member

GrabYourPitchforks commented Mar 15, 2019

The following code does not compile:

ReadOnlySpan<DayOfWeek> span1 = GetSpan1();
ReadOnlySpan<DayOfWeek> span2 = GetSpan2();

// below line results in compiler error
if (span1.SequenceEqual(span2))
{
    /* do something */
}

The reason is that the MemoryExtensions.SequenceEqual extension method is constrained to T : IEquatable<T>, but it seems like since enums are restricted to behaving exactly like their underlying primitives they should be similarly comparable. (After all, equality and comparison operators behave as expected.)

If the ConcreteEnum type implicitly implemented IEquatable<ConcreteEnum>, or if the compiler could somehow allow ConcreteEnum to fulfill the generic method constraint even if the interface isn't actually implemented, it would allow us to use enums in Span-based APIs.

@GrabYourPitchforks
Copy link
Member Author

/cc @jkotas and @CarolEidt since I heard you'd be interested in this or have thoughts on it.

@jkotas
Copy link
Member

jkotas commented Mar 16, 2019

ConcreteEnum type implicitly implemented IEquatable<ConcreteEnum>

We have done similar auto-implementation of IList<T> and friends on arrays. It was not pretty and there are still bugs around this 15 years later. The problem is that the tooling expects the metadata to be the source of truth. The moment you introduce auto-implemented interfaces like this, they have to be special cased in many places.

it seems like since enums are restricted to behaving exactly like their underlying primitives

Relaxing rules for when enums (or Spans of enums) can be converted to their underlying type would be interesting avenue to explore.

This would be as much language feature as it is a runtime feature. cc @jaredpar

@jannickj
Copy link

I just ran into this issue when using the new records in a dictionary, everything seems super nice except when you use an enum even when you mark that enum as being long / int it will still be treated as if it's impossible to compare.

What is the current best pratice way of dealing with this I am considering creating a wrapper that can cast / uncast it to an int.

@CyrusNajmabadi
Copy link
Member

@stephentoub We've talked in the past about concerns with the lang/compiler auto-implementing things. Ignoring potential breaking changes in lang behavior here, would you have any concerns if all enum types in the BCL suddenly were IEquatable?

@Joe4evr
Copy link
Contributor

Joe4evr commented Oct 29, 2020

@jannickj The best practice workaround right now would be to create a wrapper that uses EqualityComparer<T>.Default.Equals(a, b) (instead of casting) since the JIT will devirtualize that to the same asm as a == b (but only if you write it out in full).
Here's a sample you can copy that didn't even take 5 minutes to write.

@jannickj
Copy link

@Joe4evr Right but wouldn't that mean that everywhere I use Enums I'll have this ugly wrapper?

public record Data (CoolEnum Cool);

becomes:

public record Data (EnumEquatable<CoolEnum> NotCool);

@stephentoub
Copy link
Member

would you have any concerns if all enum types in the BCL suddenly were IEquatable?

I think my main concern would be on IL size increase and what impact that could have on blazor and xamarin. We recently, for example, had some PRs to reduce some use of value tuples in favor of custom structs to reduce all the extra stuff that comes with tuples. This enum change isn't as much per type, but potentially expanding many more types, and in a way that couldn't be trimmed.
CC: @marek-safar

@marek-safar
Copy link

There will be size impact from this for sure as IEquatable<> is an interface that is almost never trimmed. It would be great if we figure out a way to implement this inside Enum class or as was suggested above, extend conversion rules for enums and their underlying types.

@333fred
Copy link
Member

333fred commented Oct 29, 2020

Given that we were very concerned about the potential for silently-breaking changes with type checks on structs, I don't know how we could entertain a similar change for enums.

@dmchurch
Copy link

This is old but still open and still a thorn in lots of paws, including mine. What if, instead of auto-implementing interfaces on all enums, it were simply possible to add static and/or instance methods to enum types, thus making it possible for them to explicitly implement interfaces? That way there's no concern about breaking backwards compatibility, since it's an opt-in mechanism. I wouldn't imagine it being a particularly big problem for the runtime either, since the metadata remains the source of truth, and since enums can't be extended I wouldn't imagine there would be any issue with inheritance.

As a use case, I'm attempting to store a large number of single-byte values in a type-safe way, and in all other respects an enum derived from byte works great. I can store them in an array and they only take one byte per element, I can use Memory and Span to do range-based access and Slice to my heart's content, and I can pass elements to an overloaded function and have it choose the right overload. What I can't do is an equality comparison on a range of values, for the same reasons OP lists.

My current options are:

  1. create a holder struct, which requires a lot of boilerplate to properly act like an enum or an int and which is difficult to do cleanly, in a way that won't make the runtime deoptimize the code; see the comment in @Joe4evr's example (thank you so much for that, btw!), of // DO NOT HOIST the 'EqualityComparer' instance or the JIT can't devirt the comparison
  2. just use the raw byte type in my arrays, which forgoes type-safety and requires a whole lot of typecasts everywhere
  3. make static readonly instances of a non-enum class the classic way, which would require 8 bytes of storage for each reference on a 64-bit system, which is a huge amount of wasted memory, and in that case the compiler couldn't assume the exhaustive nature of the value list

None of those is a particularly fantastic. If I could put user-defined methods on an enum type, though, then I could simply write my own IEquatable or IComparable implementation (or, as syntactic sugar, the compiler could insert the function definitions if it sees an enum deriving from those without an explicit implementation, but it's not a big deal) and be done with it.

Note that I'm not suggesting instance fields on enums, which would (a) violate Oracle's ridiculous patent, (b) invisibly create a storage structure for the field values, which seems like a bad idea, and (c) be entirely unnecessary, given that C# has properties. Allowing method declarations for enum types would let you implement interfaces, though, and not only that, it would let you add operator overloads, or customize the string representation of ToString(), neither of which can be done in an extension class. It gives the programmer more control over type-conversion, as well, since you could declare implicit or explicit conversions to and from other classes or even other enum types - which nicely addresses the issue of any builtin or third-party enums not implementing IEquatable or whatever interface you need. After all, why should I care whether System.DayOfWeek is IEquatable if I can simply EquatableDayOfWeek day = new DateTime().DayOfWeek; without runtime cost?

Finally, adding interface and method declarations to enum types would be a great way to address @stephentoub's point about IL explosion, since IEquatable et al could be applied piecemeal across the BCL and only for classes where it might be particularly relevant (like DayOfWeek, which seems like an obvious candidate).

Are there any downsides to that solution? Because I'm not seeing any, and I'd love to know if there's something I'm missing.

@Joe4evr
Copy link
Contributor

Joe4evr commented Feb 2, 2023

If I could put user-defined methods on an enum type, though,

#297

Also, you're welcome. 🙂

@Rekkonnect
Copy link
Contributor

As already stressed out by others, enums are very ugly to use in anything outside of a simple constant collection. To enjoy more of their presence, the whole runtime must undergo several changes that begin treating enums more nicely. This includes many other convenience features, as linked above, and not just the ability to compare against equality between two numerical values that have a different name.

That being said, even after all these years of the proposal being voiced, I'm only expecting it to come around when a runtime overhaul is decided. Things will get nasty in the process of enhancing those types and it's fine, as long as they are delivered nicely and end the pain of using them.

@alrz
Copy link
Member

alrz commented Feb 23, 2023

I wonder if extensions could help here?

extension EnumExtension<T> for T : IEquatable<T> where T : Enum, struct  { .. }

@Joe4evr
Copy link
Contributor

Joe4evr commented Feb 23, 2023

I wonder if extensions could help here?

It might? It might make sense if such an extension could ship in the runtime in the same release as the feature, both as a proof-of-concept/example of what extensions can do, while at the same time not making everyone define the same extension to solve this issue (where some people would do the sub-optimal thing).

@panost
Copy link

panost commented Apr 22, 2023

You can implement it now like this
The problem to the non-inlined code is that is identical for Enums of the same size, as you can see for the examples of FileAttributes and RegexOptions. I do not know if JIT can optimize this. I mean re-use the same generated code for Enums of the same size.

@Rekkonnect
Copy link
Contributor

Your example is a known trick at least for equality comparison, but doesn't help with the general lackluster of interface implementations. And the JIT specifically optimizes the code for the frequently used types, so you'd get an optimization for additional types if you provided them in.

For enums though, you wouldn't approach their lack of correlation with INumber like that, or anything else revolving enums and enum methods. This trick is specific to implementing equality across a range of values, enums or not. Enums unfortunately don't play too nicely with anything that isn't the same enum type itself, not even their underlying integer type.

@EricOuellet2
Copy link

EricOuellet2 commented May 5, 2023

My perspective:

  • There should be a way to combines all values easily and fast (the way is up to you). That is a very specific need but I think it is highly required when using [Flags] enum in order to get filter that keep them all. See: Error CS0315 on code where it shouldn't for C# 11. It makes twisted code to my opinion where it shouldn't.
  • Being able to inherits from an Enum would be nice because we can create a derived Enum type with specifc required combinaisons without affecting the base enum values that should match a single bit ([Flags] case). But that does not meet other need where AND and OR are required.

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

No branches or pull requests