Add Enum.SetFlag and Enum.UnsetFlag

[mburbea]

Background and Motivation

Note: issue #14084, proposed this earlier but that api needed work and didn't follow the proper api-request format.
Flagged enum's have a friendly api to check for the existence of a flag, but don't currently have a counter part to set or unset a flag. I propose adding one to make it easier to work with flagged enums without resorting to writing bit manipulating expressions. When dealing with generic enums, where bit manipulation is unavailable, you'd have to write unsafe code.

Proposed API

namespace System
{
  public class Enum : ValueType, IComparable, IFormattable, IConvertible {
+    public T SetFlag(T flag, bool on = true) where T:struct, Enum;
+    public T UnsetFlag(T flag) where T:struct, Enum => SetFlag(flag, false);
  }
}

Note that since this interface would appear on all enum's, this API must throw InvalidOperationException if the instance does not match the type of T. I do not believe we should throw for enum's that do not have a FlagsAttribute.

Usage Examples

Here is an actual example from this code I wrote recently.

private InventoryFlag _flag;
public bool IsStolen
{
      get => _flags.HasFlag(InventoryFlags.IsFromStolenSource);
      set => _flags = _flags.SetFlag(InventoryFlags.IsFromStolenSource, value);
}

public bool IsUnsellable
{
    get => _flags.HasFlag(InventoryFlags.Unsellable);
    set => _flags = _flags.SetFlag(InventoryFlags.Unsellable, value);
}

Alternative Designs

The code can currently be accomplished as a one liner function for a given enum E.

// assumes that bool is always 0 or 1.
public static E SetFlag(this E @enum, E flag, bool on = true) => (@enum & ~flag) | (-Unsafe.As<bool,byte>(ref on) & flag);
// or if you can't make that assumption:
public static E SetFlag(this E @enum, E flag, bool on = true) => on ? (@enum | flag) : (@enum & ~flag);

Or as an ugly extension method that works generically. . Unfortunately, the ugly extension method does produce some extra noise at the moment.

Also, since technically, nothing is preventing this from working on bitmasks, the methods could be renamed to Enum.SetMask

Risks

For this to work well, it will likely need to be an intrinsic like Enum.HasFlag.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[GrabYourPitchforks]

@mburbea I think the comment you posted over at #14084 (comment) with this API suggestion is fine. We can keep iterating on those old threads with new ideas and bring it back for API review.

[SingleAccretion]

For this to work well, it will likely need to be an intrinsic like Enum.HasFlag.

Not necessarily, depending on how it is implemented. For example, we could make use of an internal Unsafe.As method that would strip the type from the enum and return the raw integer without bouncing through the ldarga + ldind (this would still require changes to VM code though, even if minimal...). Even the current Unsafe.As-based implementation could be improved significantly if instead of taking the address of the parameter, it would take address of the local copy.

Of course, the above would only be possible for the generic version. The object variant would most likely need special-casing in the Jit. It should not be hard though - I expect that most of the trickier logic from the current implementation of Enum.HasFlag recognition could be reused.

[mburbea]

From my testing, it seemed that the version that shows up in the method looks worse than when it is inlined at the callsite. The .net jit does a very good job with it if it can inline and creates nice and clean assembly.
(note: I had to switch the size 2 case, as it was generating pretty bad code).
as-params: https://sharplab.io/#gist:78f0d22e5f3854d02f8351004be528a0
vs
use-locals: https://sharplab.io/#gist:d0b478195bbc4c2c068a3e4b6e47c082

The diffs: https://www.diffchecker.com/MWqgZmhw
As we can see they only show up in the method body, not in the inlined property setters.

@GrabYourPitchforks, if that's the case we can close this issue.

[GrabYourPitchforks]

Dupe of #14084