Runtime: Enum Improvements

Created on 25 Jan 2017  路  64Comments  路  Source: dotnet/runtime

Enums are essential commonly used types but have several areas in need of improvement. For each non-generic Enum method there should be an equivalent generic version.

Rationale and Usage

  1. Nearly all of Enum's static methods are non-generic leading to the following issues.

    • Requires boxing for methods with enum input parameters losing type-safety, eg. IsDefined and GetName.

    • Requires casting/unboxing for methods with an enum return value, eg. ToObject and GetValues.

    • Requires the enum type to be explicitly specified as an argument.

    • Requires invocation using static method syntax.

With this proposal implemented what used to be this to validate a standard enum value

```c#
MyEnum value = ???;
bool isValid = Enum.IsDefined(typeof(MyEnum), value);

now becomes this

```c#
MyEnum value = ???;
bool isValid = value.IsDefined();

With this implemented it will address dotnet/runtime#18063.

Proposed API

```c#
namespace System {
public abstract class Enum : ValueType, IComparable, IConvertible, IFormattable {
public static TEnum Parse(ReadOnlySpan value) where TEnum : struct, Enum;
public static TEnum Parse(ReadOnlySpan value, bool ignoreCase) where TEnum : struct, Enum;
public static object Parse(Type enumType, ReadOnlySpan value);
public static object Parse(Type enumType, ReadOnlySpan value, bool ignoreCase);
public static bool TryParse(ReadOnlySpan value, out TEnum result) where TEnum : struct, Enum;
public static bool TryParse(ReadOnlySpan value, bool ignoreCase, out TEnum result) where TEnum : struct, Enum;
public static bool TryParse(Type enumType, ReadOnlySpan value, out object result);
public static bool TryParse(Type enumType, ReadOnlySpan value, bool ignoreCase, out object result);

    // Generic versions of existing methods
    public static string GetName<TEnum>(this TEnum value) where TEnum : struct, Enum;
    public static IReadOnlyList<string> GetNames<TEnum>() where TEnum : struct, Enum;
    public static IReadOnlyList<TEnum> GetValues<TEnum>() where TEnum : struct, Enum;
    public static bool IsDefined<TEnum>(this TEnum value) where TEnum : struct, Enum;
    public static TEnum ToObject<TEnum>(object value) where TEnum : struct, Enum;
    public static TEnum ToObject<TEnum>(sbyte value) where TEnum : struct, Enum;
    public static TEnum ToObject<TEnum>(byte value) where TEnum : struct, Enum;
    public static TEnum ToObject<TEnum>(short value) where TEnum : struct, Enum;
    public static TEnum ToObject<TEnum>(ushort value) where TEnum : struct, Enum;
    public static TEnum ToObject<TEnum>(int value) where TEnum : struct, Enum;
    public static TEnum ToObject<TEnum>(uint value) where TEnum : struct, Enum;
    public static TEnum ToObject<TEnum>(long value) where TEnum : struct, Enum;
    public static TEnum ToObject<TEnum>(ulong value) where TEnum : struct, Enum;
    public static int CompareTo<TEnum>(TEnum value, TEnum other) where TEnum : struct, Enum;
    public static bool Equals<TEnum>(TEnum value, TEnum other) where TEnum : struct, Enum;
    public static byte ToByte<TEnum>(TEnum value) where TEnum : struct, Enum;
    public static short ToInt16<TEnum>(TEnum value) where TEnum : struct, Enum;
    public static int ToInt32<TEnum>(TEnum value) where TEnum : struct, Enum;
    public static long ToInt64<TEnum>(TEnum value) where TEnum : struct, Enum;
    public static sbyte ToSByte<TEnum>(TEnum value) where TEnum : struct, Enum;
    public static string ToString<TEnum>(TEnum value) where TEnum : struct, Enum;
    public static string ToString<TEnum>(TEnum value, string format) where TEnum : struct, Enum;
    public static ushort ToUInt16<TEnum>(TEnum value) where TEnum : struct, Enum;
    public static uint ToUInt32<TEnum>(TEnum value) where TEnum : struct, Enum;
    public static ulong ToUInt64<TEnum>(TEnum value) where TEnum : struct, Enum;
}

}
```

API Details

This proposal makes use of a C# language feature that needs to be added in order for this proposal to make the most impact.

This proposal specifies extension methods within System.Enum and as such requires C# to allow extension methods within non-static classes as is proposed in csharplang#301. Promoting these to extension methods later would be a breaking change due to csharplang#665 but I feel this is acceptable.

Alternatively, the extension methods could be defined in a separate static EnumExtensions class. This is uglier but would avoid this issue and the extension methods would be available immediately instead of needing to wait for a later C# version to support this.

Open Questions

  • Is promoting the System.Enum extension methods later an acceptable breaking change due to csharplang#665? If not should we wait for a later C# version that supports extension methods in System.Enum or should we introduce a separate EnumExtensions class?

Implementation Details

This proposal stems from my work on the open source library Enums.NET which addresses each of these issues and will be the basis for an implementation. For efficiency this is how Enums.NET is implemented.

First, using Enum as a generic type argument causes generic code-explosion in today's runtimes so it's essential that it's included sparingly. For this reason most logic is contained in an EnumCache<TInt, TIntProvider> object where TInt is the underlying type and TIntProvider is the underlying type's operations provider which implements the interface INumericProvider<TInt> which provides the bitwise operations and other needed operations. Including the TIntProvider as a type parameter allows the runtime to inline calls to its methods so there's no interface method dispatch, a technique described in this generic calculations article.

Now how can one access this EnumCache object when the caller only has the enum type as a generic type argument? This is solved by the bridge-like object EnumInfo<TEnum, TInt, TIntProvider> which implements the interfaces IEnumInfo<TEnum> for generic methods and IEnumInfo for non-generic methods. It simply acts as a bridge to delegate calls from the interfaces to EnumCache<TInt, TIntProvider>. In order to do that it needs to be able to efficiently convert values between TEnum and TInt. This is performed by the conversion methods ToInt and ToEnum which are declared as extern methods for efficiency so that they're simply defined as casts from one type to the other. These method's functionality can be achieved with the Unsafe.As method. This EnumInfo<TEnum, TInt, TIntProvider> is created via reflection upon it's first call and is stored in the static field Enums<TEnum>.Info as an IEnumInfo<TEnum> which is then called by Enums's generic methods.

For non-generic methods the RuntimeType.GenericCache property will be set to the IEnumInfo to use.

Updates

  • Added more implementation details.
  • Added more API details.
  • Added enumerating members usage example.
  • Added generic versions of instance methods for performance reasons.
  • Removed the System.Enums namespace and moved its members into the System namespace.
  • Moved PrimaryAttribute into the System.ComponentModel namespace and switched to using the existing System.ComponentModel.AttributeCollection.
  • Removed IsValid method as it's meaning is rather subjective and promoted IsValidFlagCombination as an extension method.
  • Renamed GetAllFlags to AllFlags as it more closely resembles a property but can't be because it is generic. Similar naming to Array.Empty.
  • Removed EnumComparer as one can just use Comparer<T>.Default and EqualityComparer<T>.Default instead.
  • Split the proposed API to a base API addition and an EnumMember support API addition.
  • Added ReadOnlySpan<char> parse overloads.
  • Moved FlagEnum.IsFlagEnum into System.Enum.
  • No longer proposes adding the Enum constraint to existing methods.
  • Split EnumMember API addition into the separate issue dotnet/corefx#34077.
  • Split generic API additions that don't necessitate the performance improvements of this implementation to separate issue dotnet/runtime#2364.
  • Split Flag API additions to separate issue dotnet/corefx#34079.
  • Split PrimaryAttribute addition to separate issue dotnet/corefx#34080.
  • Moved Format to dotnet/runtime#2364 as well.
  • Moved proposed methods from dotnet/runtime#2364 back to this issue.
  • Removed Format as it's no different from ToString(string format) except it throws an ArgumentNullException for a null format parameter.
Design Discussion api-needs-work area-System.Runtime

Most helpful comment

VS 15.7 is live and with it so is C# 7.3's support for the Enum constraint. Now that that's a reality, I'd love for this proposal to be pushed to target a release in .NET Core 2.2! Let's make this happen. Let me know what more this proposal needs to be reviewed. Thanks.

All 64 comments

Updated the proposal to include some more implementation details.

@danmosemsft I've updated the proposal to include more details about the new API defined.

The second one is that this proposal specifies extension methods within System.Enum and as such requires C# to allow extension methods within non-static classes, a proposal of which needs to be worked up.

There already is a proposal for that too: https://github.com/dotnet/roslyn/issues/16271.

@svick Thanks, I've updated the proposal accordingly.

I've updated the proposal to include generic versions of instance methods for performance reasons.

@danmosemsft are you the CoreFX owner of this proposal. If so, could you let me know what needs to be done to get this API reviewed?

@joperezr @AlexGhiondea are the owners of this area so one of them will either give feedback on it or flip to "api-ready-for-review".

Thanks @danmosemsft.

I've updated the proposal by removing the System.Enums namespace and moved its members into the System namespace. Does anyone think ReadOnlyAttributeCollection and PrimaryAttribute should be included in another namespace, maybe System.ComponentModel?

I see that there is already an AttributeCollection in the System.ComponentModel namespace which would suit my needs with some additions but it's included in the System.ComponentModel.TypeConverter assembly. Are binding redirects possible in .NET Core to enable moving this AttributeCollection class into mscorlib?

If it's an interesting type that we could use as is (or modify without breaking existing users) then it's possible for us to move it down so this code could depend on i.

Excellent, I'll update my proposal accordingly.

@joperezr @AlexGhiondea I believe I'm done with my proposal updates. When you have some time could you please look over this proposal?

I like the proposal a lot.
One api that I'm missing is a method that remove not valid flags.

public static TEnum RemoveNotValidFlags(this TEnum value) where TEnum : struct, Enum;

See dotnet/runtime#18190

For that case I'd suggest doing this.

c# myFlagEnumValue = myFlagEnumValue.CommonFlags(FlagEnum.AllFlags<MyFlagEnum>());

I know it's less convenient but it seems too much of a specialized case to warrant a new extension method.

Maybe as just a static method we could have the following.

```c#
public static TEnum GetValidFlags(TEnum value) where TEnum : struct, Enum;

and in use would be

```c#
myFlagEnumValue = FlagEnum.GetValidFlags(myFlagEnumValue);

Thoughts, @magol?

You right, maybe it is not a good method for a General api arter all. 鈽猴笍

@TylerBrinkley thanks for putting this together. It is a very interesting proposal that I am trying to better understand.

I have a few questions

  • This requires 2 language features to enable if I read it correctly. Does that mean it won't be possible to even implement this until those features are shipped.

    • I am trying to understand how I would use the GetMember methods -- that looks like an object model on top of an enum value -- and I understanding it wrong?

    • Why are there both generic and non-generic versions of some APIs?

    • What are your thoughts on implementing this? Have you looked at the implementation in CoreCLR for the Enum type? I believe quite a few tricks and calls into runtime and JitHelpers are used to make the enum work and have reasonable performance.

/cc @jkotas

@AlexGhiondea thanks for looking at the proposal.

  • This proposal makes use of 2 language features that haven't been implemented yet but they are not required in order to implement this proposal. Essentially, the proposed extension methods would be implemented as static methods initially and so users would need to use static method invocation syntax to use them. When these 2 language features are shipped those static methods would then be updated to become extension methods.
  • No you're absolutely right. EnumMember is an object model on top of an enum member's name, value, and attributes and enables retrieving them at the same time such as in the second use case example above using GetMembers. It is not an object model on top of any enum value though, only those values that are defined hence the inclusion of the name and attributes.
  • Non-generic API's are still very useful for implementing things like serializers and code generators. The non-generic TryParse was added for similar reasons in dotnet/runtime#14083.
  • The implementation would be based on Enums.NET's implementation which has great performance as can be seen here and would not require all those tricks and calls into the runtime and JitHelpers. A brief explanation of its implementation details can be seen in the Implementation Details section on the proposal. The only part I'm not sure how to implement would be the ToInt and ToEnum extern methods which are simply defined as casts from one type to the other. In Enums.NET I use Fody to implement these methods directly using IL. How does CoreCLR implement extern methods?

Enums.NET's implementation which has great performance

It may have good enough performance for some uses, but it does not have what I would consider great performance. Great performance would mean parity when you do the same with bitmask operations.

How does CoreCLR implement extern methods

The JitHelpers are other tricks that @AlexGhiondea is talking about... .

@TylerBrinkley

This proposal makes use of 2 language features that haven't been implemented yet but they are not required in order to implement this proposal. Essentially, the proposed extension methods would be implemented as static methods initially and so users would need to use static method invocation syntax to use them. When these 2 language features are shipped those static methods would then be updated to become extension methods.

That's true for the "extension methods in non-static classes" feature, but not for "enum as generic constraint". I think that adding that constraint later (the proposal even adds it to some existing methods) would be a breaking change. And I think the break is not just theoretical, but that it would break real code that has a generic method that calls one of the existing generic methods, something like:

c# T MyMethod<T>() where T : struct => Enum.Parse<T>(GetEnumNameSomehow());

I can see two ways out of this:

  1. Don't add the constraint now and keep it that way forever.

  2. Add the constraint to the new methods now (without C# support, that would probably require implementation in IL or some kind of post-compile IL rewriting). Keep existing methods without the constraint.

    This assumes that consuming the constraint from C#, VB and F# already works fine (which I have not checked).

Neither option sounds great, but 1 is probably more practical, assuming it does not make sense to wait until enum constraints arrive in C# (which may never actually happen).

@jkotas

Great performance would mean parity when you do the same with bitmask operations.

From my performance testing of Enums.NET's HasAllFlags method's performance comes very close to doing the same with bitmask operations, much more compared to the already implemented HasFlag method. I just ran the performance on the .NET Framework on my computer with this testing method and HasFlag took 959 ms, HasAllFlags took 139 ms, and manually using bitmask operations took 83 ms. I think the only performance overhead in using HasAllFlags over manually using bitmask operations is due to the interface dispatch call internally to IEnumInfo<TEnum>.

I've added a little more details to the implementation details of the proposal regarding the use of the generic type parameter TIntProvider and how it's operations including the bitwise operations are being inlined.

Runtime throughput is not the only performance metric we are concerned about. The startup and code bloat is another one - using generic helper methods for bitmasks checks is many times (100x?) worse on this performance metric.

@svick

That's a great point. You're right, it would be a breaking change if another generic method with a T : struct constraint currently calls Enum.Parse or Enum.TryParse. If the constraint on Enum.Parse and Enum.TryParse aren't changed it'll just mean there'll be a runtime type check which wouldn't be a big deal as parsing should take orders of magnitude more time to evaluate. While I think breaking changes should be avoided at all costs I think this might be a case for an exception as its impact should be very limited given the existing T : struct constraint. My guess would be most users who may be affected by this want their generic parameter constrained to T : struct, Enum anyway which will be available to them at the same time, at least in the C# world.

In regards to the implementation of these new methods now, option 1 is not really an option as that would mean no extension methods and would mean runtime type checks which would greatly decrease performance for the bitmask operation methods which should be near native in performance. If the breaking change is not acceptable I'm in favor of option 2 if possible or else wait for C#'s Enum type constraint.

This assumes that consuming the constraint from C#, VB and F# already works fine (which I have not checked).

Consuming the Enum constraint from C#, VB, and F# does already work.

Is it possible to do any optimization by roslyn som that the calls to this method is replace with native bit operations at compile time?

@magol that should be possible similar to what's proposed for ToString

As a proof of concept I've started migrating my code into mscorlib at TylerBrinkley/coreclr.

@TylerBrinkley thanks for the update! I would be interested to learn some numbers once you have your proof of concept operational.

@AlexGhiondea Currently, Enum.ToObject will accept overflow values. Am I correct in assuming that that behavior must be preserved? In Enums.NET I throw an OverflowException in those cases. It seems more like a bug that it allows overflowing values.

@TylerBrinkley based on the MSDN documentation it looks like we make the call to not throw if the value overflows. You can call IsDefined to see if you can fit the value in the underlying type.

If we are going for a drop-in replacement, then we should respect this behavior. If we are going with a new type... then we can do something else.

I opened a proposal just for Enum.IsDefined here: https://github.com/dotnet/corefx/issues/23240. That seems to be the most valuable thing in all of this proposal. The other stuff will take ages to get through and does not seem like it would be used as frequently.

Thanks for that @jamesqo we will take a look at it in our next triage.

When using generics, we have to use DynamicInvoke, boxing, and EnumParse when converting with enums. This is especially true of framework code that aims to be 'user friend;y' and let consumers define their own types. (See MVVM / RX type programming)

This activity causes performance issue, especially on mobile devices.

If I could implicitly cast Enums to/from int without the boxing, dynamic invocation, and conversion I would be happy.

````
// Replace This
void Convert(TValue val)
{
int b = Convert.ToInt32(val);
}

// With This
void Convert(TValue val) where TValue : enum, int
{
int a = val;
}

// Replace This
void Convert(int val)
{
var a = (TValue)Enum.Parse(typeof(TValue), val);
}

// With This
void Convert(int val) where TValue : enum, int
{
var a = (TValue)val;
}

````

Updated examples, my bad.

@NVentimiglia what's the issue with the first one you propose replacing with a Convert? It should just be a type-coercing stloc/ldloc at most, and likely just a nop that depends on the use of the int coercing the type.

@JonHanna

Thanks, I think I might be confused. If there is no penalty for converting enums to/from ints, why do libraries such as Enums. Net exist ?

@NVentimiglia There isn't except when the type of the enum is generic.

@jkotas We should consider exposing UnsafeEnumCast as a static method on Enum.

@NVentimiglia mostly as a replacement of the Enum static methods that go via object and hence do involve boxing. The direct cast of int a = (int)MyEnum.Green; doesn't.

If you compile something like public static int Add(StringComparison x, StringComparison y) => (int)x + (int)y; in a release build you'll see that it gets compiled to:
CIL ldarg.0 ldarg.1 add ret
Just as if the arguments had been int.

Cases where you can't just do a direct cast though, are another matter.

@jkotas

There isn't except when the type of the enum is generic.

Thanks, in my case I am using generics.

@JonHanna

mostly as a replacement of the Enum static methods that go via object and hence do involve boxing. The direct cast of int a = (int)MyEnum.Green; doesn't.

Thanks, in my case I am using generics, so I am using the static helper methods.

Sorry, for the confusion, I should of been more clear in my comment, will update.

@jamesqo UnsafeEnumCast would not be necessary since this proposal includes generic ToInt32, ToInt64, etc. methods.

Do you think it possible to compile-time constant fold the setting and clearing of enum flags?

I suppose it's possible either Roslyn and/or RyuJit could be updated later to change calls to the flag methods to use the bitwise operators directly.

Conseder adding a new Enum Type called Flag as well dotnet/corefx#27110

I suggest Generic version of IsEnum method should be implemented in Enum class in addition to all Enum improvements:

public static bool IsEnum() where TEnum : struct
{
return typeof(TEnum).GetTypeInfo().IsEnum;
}

I suggest some improvement for initial proposals in this issue.
I guess methods such as

// Generic versions of existing methods
public static IEnumerable<TEnum> GetValues<TEnum>() where TEnum : struct, Enum;

should contain result's type IReadOnlyList<TEnum> or IImmutableList<TEnum> instead IEnumerable<TEnum> to easy access to elements Count, and enable to access elemens by index cause current GetValue version returns elements in sorted order:

// Generic versions of existing methods
public static IImmutableList<TEnum> GetValues<TEnum>() where TEnum : struct, Enum;

What benefit would there be in providing a generic IsEnum method on Enum?

I like the idea of returning an IReadOnlyList for GetValues, GetNames, and GetMembers but would have to evaluate how to achieve that while not introducing any performance issues such as allocations or duplicating much static enum data.

VS 15.7 is live and with it so is C# 7.3's support for the Enum constraint. Now that that's a reality, I'd love for this proposal to be pushed to target a release in .NET Core 2.2! Let's make this happen. Let me know what more this proposal needs to be reviewed. Thanks.

@terrajobst thoughts about readiness of this API proposal for review?

Enum.HasFlag wouldn't need to be marked obsolete now its a Jit intrinsic?

dotnet/coreclr#5626
dotnet/coreclr#13748

I still think Enum.HasFlag should be obsoleted as it's ambiguous and clutters up intellisense as it's available on all enum values, flag or not, as it's an instance method. Also, if this makes it's way to .NET Standard there's no telling whether the consumer's runtime treats it as a JIT intrinsic.

Enum.HasFlag is really poor API and we should offer something better, having runtime intrinsic still does not address the design issue where for example following code compiles but it rather shouldn't

```c#
DateTimeKind.Local.HasFlag (AttributeTargets.Class);

Unfortunatelly, by only introducing a new extension method like 

```c#
    public static bool HasFlag<T> (T enumType, T enumValue) where T : Enum
    {
        return default;
    }

won't work because existing HasFlag instance method will take precedence in most languages.

That won't be an issue as this proposal adds the following extension methods with different and less ambiguous names.

c# public static bool HasAllFlags<TEnum>(this TEnum value, TEnum flags) where TEnum : struct, Enum; public static bool HasAnyFlags<TEnum>(this TEnum value, TEnum flags) where TEnum : struct, Enum;

@terrajobst as @danmosemsft asked, what are your thoughts about the readiness of this API proposal for review now that Enum is an allowed C# constraint?

I've updated the proposal to add support for ReadOnlySpan<char> parse overloads. I've also re-implemented the Base API changes at TylerBrinkley/coreclr. Everything's implemented, albeit untested, not sure how to proceed with that.

@terrajobst and @danmosemsft would it be better for me to break this proposal up into smaller chunks so that it could be api reviewed?

I've moved the EnumMember API addition to dotnet/corefx#34077.

I've moved the generic API additions that don't necessitate the performance improvements of this implementation to dotnet/runtime#2364.

I've moved the flag API additions to dotnet/corefx#34079.

I've moved the PrimaryAttribute addition to dotnet/corefx#34080.

This ticket will remain for a few performance API's and as the basis for the improved performance implementation.

I've implemented the generic implementation proposed here without any API changes in coreclr#22161.

Would it be doable to have TryParse overloads that take an argument that makes it return false if the string or span is integral?

I can think of two variants, 1) one where any integral value would be invalid, because it is meaningless. 2) The other is to shoot down invalid values, such as TryParse<ConsoleColor>("16", ...)

Ad 1) in my opinion it is fairly common to define some valid input that should be constrained to known strings as an enum. All that is probably needed is to check whether the first character (if any) is a letter or underscore. Especially for technical apps it is very common to have invariant strings that need to be parsed and to define those as enums. E.g. suppose a console application that takes a PlatformID as an argument should be able to use one of these overloads to shoot down 2 although it would be Win32NT.

For variant 2, having to parse first and then check with something like IsDefined is in fact for the most part double work.

Would it be doable to have TryParse overloads that take an argument that makes it return false if the string or span is integral?

That would be outside the scope of this issue so perhaps you may want to open a new one?

You can achieve this today with Enums.NET by specifying only EnumFormat.Name.

Kudos for this for starters - I just bumped into having to do ReadOnlySpan<char>.ToString() to get an enum parsed ;)

What's about implementing IEquatable<T> and IComparable<T> on enums? It looks not so simple, because enums do have metadata, unlike arrays being pure runtime-magic.
Or skip it for 1 year and jump to type classes, which is planned for C# 10 and I assume to come with .NET 6?

@TylerBrinkley I'm not sure which approach you've taken to implement this but I just posted this comment which details a way to implement these operations with full bitwise operator speed. I thought I would mention it here as it turns out this appears to be the main discussion area for these.

When the methods are written like my example, they get inlined directly to bitwise operations and the branches are eliminated by the JIT so it produces very efficient code.

I'm not sure if you're interested in additional related features but I'd love to see support added to be able to enumerate values or names in the order they are defined (likely with an optional parameter on the GetNames and GetValues methods) since the current methods only enumerate in ascending value.

Ha! It's funny to see that almost everyone implemented their well-performing enum library when it used to be so slow in the .NET Framework era. So did I, of course. :)

I'm really happy to see the improvements made in .NET Core. I would've never created my alternative version if System.Enum was always that fast. In fact, I needed to do a massive refactoring in my libraries, just to justify its today's existence and make it faster again than the Core version. :) But to be frank, maybe its last really useful feature that it supports ReadOnlySpan<char>, which is still not implemented in the System version.

Anyhow, if it helps anything, feel free to re-use any solutions from it. Maybe it hasn't as many features as Tyler's version and my approach was also much simpler in some aspects, but who knows... maybe you can find some useful parts in it.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jchannon picture jchannon  路  3Comments

jkotas picture jkotas  路  3Comments

bencz picture bencz  路  3Comments

nalywa picture nalywa  路  3Comments

matty-hall picture matty-hall  路  3Comments