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

Champion "Allow System.Enum as a constraint" (15.7) #104

Open
1 of 5 tasks
gafter opened this issue Feb 14, 2017 · 136 comments
Open
1 of 5 tasks

Champion "Allow System.Enum as a constraint" (15.7) #104

gafter opened this issue Feb 14, 2017 · 136 comments
Assignees
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion
Milestone

Comments

@gafter
Copy link
Member

gafter commented Feb 14, 2017

Allows where T : System.Enum

  • Proposal added
  • Discussed in LDM
  • Decision in LDM
  • Finalized (done, rejected, inactive)
  • Spec'ed

See also dotnet/roslyn#262

@gafter gafter changed the title Generic constraint "enum" (or allow System.Enum as a constraint) Champion Generic constraint "enum" (or allow System.Enum as a constraint) Feb 15, 2017
@TylerBrinkley
Copy link

TylerBrinkley commented Feb 16, 2017

I'm of the opinion that Enum was not allowed as a generic constraint initially in C# due to generic code-explosion as the current .NET runtimes don't share generic code for value types like they do for reference types. With the implementation of corefx#15453 this would be less of a problem as it should cover most use cases for an Enum generic type constraint and internally it would try to reduce this generic code-explosion by limiting usage of Enum as a generic type parameter as much as possible. I'm wondering if something could be done in the runtime to allow types and methods with generic enum type arguments to share code when the enum type shares the same underlying type.

Perhaps another reason Enum was not allowed as a generic constraint initially in C# was due to the potential expectation of users that the bitwise operators would be available on those generic types for flag operations. Implementing these bitwise operators were discussed in roslyn#262 and even before that on Roslyn's CodePlex site and the consensus was that there were issues when trying to implement them in IL due to differing underlying types. This expectation would be mitigated by the implementation of the flag enum operations provided in corefx#15453.

I may be mistaken but I believe removing the explicit filters in the compiler preventing Enum as a generic constraint would allow it to just work as expected as any other class type constraint.

I would love to see this implemented as soon as possible to enable the benefits corefx#15453 would provide. I'd gladly contribute in anyway needed.

@gafter gafter changed the title Champion Generic constraint "enum" (or allow System.Enum as a constraint) Champion "Generic constraint enum (or allow System.Enum as a constraint)" Feb 21, 2017
@gafter gafter added this to the X.X candidate milestone Feb 22, 2017
@temporaryfile
Copy link

Generic constraints make sense with enum inheritance. Example from the future:

public enum WindowMessage
{
    WM_NULL = 0,
}

public enum ListViewMessage : WindowMessage
{
    LVM_FIRST = 0x1000,
    LVM_EDITLABELW = (LVM_FIRST + 118),
}

public static IntPtr SendMessage<TMessageType>(
    HWND hwnd,
    TMessageType Msg = WindowMessage.WM_NULL,
    UIntPtr wParam = default(UIntPtr),
    IntPtr lParam = default(IntPtr))
    where TMessageType : WindowMessage
{
    // [....]
}

@TylerBrinkley
Copy link

TylerBrinkley commented Aug 16, 2017

@gafter & @AnthonyDGreen is there anything myself or the community can do to make this a reality? I'd really like to move forward with corefx#15453 but this seems to be the blocker.

I believe the only code changes needed would be a few line removals in roslyn/Binder_Constraints.cs.

@gafter
Copy link
Member Author

gafter commented Aug 16, 2017

@TylerBrinkley I don't see what aspect of the proposed API would benefit from this.

@TylerBrinkley
Copy link

TylerBrinkley commented Aug 17, 2017

@gafter There are two issues with implementing corefx#15453 now without the generic Enum constraint.

  1. Adding the Enum constraint later to enable extension methods would be a breaking change and may not be acceptable. In the proposal I've added the constraint to the existing generic versions of Enum.Parse and Enum.TryParse but that would be a breaking change as noted in this comment. If we can't add the Enum constraint to those two methods it wouldn't be a big deal as all it may mean is an additional type check but for the other methods proposed it would be detrimental as they couldn't be made into extension methods.
  2. Even if adding the Enum constraint later is deemed an acceptable breaking change the FlagEnum methods cannot later be promoted to extension methods due to Static using makes extension method promotion a breaking change #665.

@TylerBrinkley
Copy link

@gafter or @AnthonyDGreen again I ask, is there anything myself or the community can do to make this a reality?

@TylerBrinkley
Copy link

Forgive me but I'm going to continue to spam this request until I get a response.

@gafter or @AnthonyDGreen is there anything myself or the community can do to make this a reality?

@jnm2
Copy link
Contributor

jnm2 commented Sep 22, 2017

Ditto.

@TylerBrinkley
Copy link

@MadsTorgersen I'm sure you get endless GitHub mentions so I apologize upfront but I was hoping you could review this. We've been trying to get a response from @gafter or @AnthonyDGreen for awhile now on how to proceed with this request but without success.

I'm the author of the OSS library Enums.NET and have created a feature request in the corefx repo to include many of the enhancements my library provides directly to .NET Core. The one thing holding that request back is this missing C# language feature. Is there anything myself or the community can do to push this along? Thank you for your time.

@gafter gafter modified the milestones: X.X candidate, 7.X candidate Oct 4, 2017
@gafter
Copy link
Member Author

gafter commented Oct 4, 2017

I snuck this into the 7.x milestone to force the LDM to triage this next week.

@TylerBrinkley
Copy link

Thank you so much @gafter!

@MadsTorgersen MadsTorgersen modified the milestones: 7.X candidate, 7.3 candidate Oct 9, 2017
@panost
Copy link

panost commented Nov 9, 2017

@TylerBrinkley
How about if you limit the enum scope by specifying the underlying type also ?
for example

 where T : enum, int
or
 where T : enum(int)

and T will accept any Enum that has int as underlying type

That will enable to use addition or bit operations and also the code can be shared

That accompanied with ref extensions methods will enable us to write any Enum flag operation possible with few generic methods

public static void SetFlag(ref this T baseVal, T flag, bool value) where T : enum(int)
public static void SetByteFlag(ref this T baseVal, T flag, bool value) where T : enum(byte)

Edit: DOH!! It appears that my suggestion was first proposed by HaloFour two and a half years ago and not only that, it appears that F# already has enum generic constraints with underlying type. I am sorry about that..

@tarrowood-imangi
Copy link

Glad to see that after about a decade of complaints and hacked workarounds, there's finally some semblance of movement on this. I've lost count of how many times I've needed this, it's an extreme limitation when trying to do any kind of generic GUI programming. Thanks, @TylerBrinkley, for being the squeaky wheel here.

@NVentimiglia
Copy link

NVentimiglia commented Dec 6, 2017

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>(TValue val)
{
   int b = Convert.ToInt32(val);
}

// With This
void Convert<TValue>(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<TValue>(int val) where TValue : enum, int
{
     var a = (TValue)val;
}

Updated examples, my bad.

@HaloFour
Copy link
Contributor

HaloFour commented Dec 6, 2017

@NVentimiglia

I don't understand what you're asking. Explicitly casting to/from an enum and it's underlying integral type (which is not necessarily int or compatible with int) is a non-op. Adding helper methods into the mix would only add overhead.

@NVentimiglia
Copy link

@HaloFour What helper method am I suggesting ?

@HaloFour
Copy link
Contributor

HaloFour commented Dec 7, 2017

@NVentimiglia

That Convert<T> method you described. Why would that be better than just explicitly casting MyEnum to int? And you don't need Enum.Parse to convert from int to MyEnum. Just cast.

@NVentimiglia
Copy link

NVentimiglia commented Dec 7, 2017

@HaloFour

  • I would like an implicit cast (enum as int) so I do not need to box cast. The box casting is a heap allocation and a performance hit.
  • without generic constraints I can only handle enums as objects (heap) (thus the use of the current helper methods, which I loath).
  • A generic constraint on the method is a prerequisite for my efficiency goals.

@HaloFour
Copy link
Contributor

HaloFour commented Dec 7, 2017

@NVentimiglia

Expliciting casting an enum to an int (or back) does not involve boxing.

Yes, an enum constraint will help when writing generic helper methods. Note that there are no generic constraints to allow you to constrain the underlying type of the enum (e.g. int) so any generic method is still going to have to deal with the fact that T could be any size and you can't do direct arithmetic operations on it. I don't see why these generic methods required doing allocations today, though.

@HaloFour
Copy link
Contributor

HaloFour commented Jun 27, 2018

@panost

Name one runtime feature of C# lang since version 5.

Default Interface Members will require a runtime feature and that's currently being prototyped. There's a chance that it won't happen as both teams are loathe to rev the runtime for a language feature, but that's one case where it's being considered due to a lack of options to solve the problem in the language. There is the argument that while the hood is up other minor changes might be considered.

Also, IIRC, the verifier was relaxed to better support ref returns/locals. So it's possible that the verifier could be relaxed to also allow add (and other arithmetic opcodes) for T where T is constrained to both struct and System.Enum.

@CyrusNajmabadi
Copy link
Member

You will get different results,

Note if the runtime collapses 'enum constrained' type parameters to actually just be their underlying type. That was the point i was making. It was talking about why the arbitrary 'collapsing' proposal wouldn't be acceptable

and in the case of Enums could create the following

yes. this is exactly what i was proposing here: #104 (comment)

I think that their is some optimization that eliminates the additional methods, if the generic method is static or sealed and doesn't use typeof, but I can't find a link to it.

Great. All of that is still at teh CLR level. None of this involves csharplang being involved :) It's still unclear to me why there is any discussion going on here about this :)

@CyrusNajmabadi
Copy link
Member

@panost Can you be specific: what change are you asking about for the C# language itself.

Note that the C# language itself does not care about how any particular underlying platform implements generics. That's an implementation detail. There could be some platforms that literally generate a different impl for each instantiation. There could be some platforms that literally generate a single impl for all instantiations. C# doesn't care. C# just defines what you are allowed to say in C# and what the semantics are of that.

If you want the actual low level implementation fo generics to be better (i.e. generating better code for enums), then that isn't under the purview of C#-lang. So, again, to repeat, here's the flowchart:

  1. are you asking to actually change the C# language at all? i.e. new syntax, or new semantics? or a change in something that already exists at the langauge level? If so, this is the right repo.
  2. are you asking the MS c# compiler to emit different IL for some language construct? i.e. maybe there's a more optimal way to emit some language construct. Or the runtime added new instructions and you want the C# compiler to use that. Then that's dotnet/roslyn
  3. if you want better runtime codegen, or new facilities in the runtime (i.e. a way to express enum constraints differently), then that's going to be dotnet/coreclr

@panost
Copy link

panost commented Jun 27, 2018

I want this to compile and of course be usable :) by any enum.

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void SetFlag(ref this T baseVal, T flag, bool value) where T : enum {
  if (value) {
   baseVal |= flag;
  } else {
    baseVal &= ~flag;
  }
}

@yaakov-h
Copy link
Member

(Did you forget a <T>?)

@CyrusNajmabadi
Copy link
Member

@panost It feels like these methods would be written extremely rarely. And there are suitable ways to write them now that have been shown to be quite fast. Are there reasons the existing solutions are not suitable?

@panost
Copy link

panost commented Jun 28, 2018

@yaakov-h Yes it needs a <T>
@CyrusNajmabadi That was an answer to your question of what change I want to the C# lang

Yes these methods would be written extremely rarely, I can thing of a maximum of 20 [Flags]Enum-oriented methods, but the "technologies" behind, that used to achieve that, are going to be used extensively. For example the INumber or IBitwise "shapes" (or whatever they called now).

I am not familiar with the Unsafe methods of @ufcpp, I need to investigate what exactly gets inlined. I also think that they are a solution for NETCore only. Note that @ufcpp did not include NETFramework benchmarks. But the fact remains that this implementation doesn't clearly reflect the developer intentions, or to put it otherwise is a hack to overcome a language limitation.

@HaloFour I prefer to allow bitwise ops, because Enum "satisfies" (or implements) the IBitwise shape.

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void SetFlag<T>(ref this T baseVal, T flag, bool value) where T : IBitwise {
  if (value) {
   baseVal |= flag;
  } else {
    baseVal &= ~flag;
  }
}

But honestly I don't believe that any of those changes (Default Interface Members included) will come any time soon. Checking the C# history further, it appears that C# 7.3 could be used with a few external assemblies and some changes to the verifier, by NET framework 2.0 (released in 2005) without any trouble. It appears that C# lang has this limp for quite some time.

@thomaslevesque
Copy link
Member

I realize it's a bit late, now that the feature is released, but I'm wondering two things:

  • why can't we use enum (the keyword) instead of Enum (the type) as a constraint?
  • why doesn't the Enum constraint imply the struct constraint, since all enums are value types?

@ufcpp
Copy link

ufcpp commented Aug 10, 2018

using System;

class Program
{
    static void X<T>(T x) where T : Enum { }

    static void Main()
    {
        PlatformID x = PlatformID.Win32NT;
        X(x);
        X<Enum>(x); // x is boxed to Enum class
    }
}

@HaloFour
Copy link
Contributor

@thomaslevesque

why can't we use enum (the keyword) instead of Enum (the type) as a constraint?

Mainly to lift the imposed limitation without precluding the possibility of more enhancements in the future.

why doesn't the Enum constraint imply the struct constraint, since all enums are value types?

Except for System.Enum which is a reference type.

The goal was to keep it simple. Remove a line from the spec, remove a condition in the compiler and done. Anything above and beyond that would require further design and spec work. With the desire to support bitwise ops on enums this could've got lost in the quagmire.

@thomaslevesque
Copy link
Member

I see, thanks for the explanation.

@tomkerkhove
Copy link
Member

I would like to use Enum constraint with flags capabilities as well.

@PathogenDavid
Copy link

I believe a flags constraint would run into the same issues as a numeric constraint. The IL instructions emitted depend on the underlying type of the enum, so you couldn't emit code for that sort of constraint today.

@IanKemp
Copy link

IanKemp commented Nov 2, 2018

This was shipped with C# 7.3, so shouldn't this issue be closed?

@gafter
Copy link
Member Author

gafter commented Nov 2, 2018

@IanKemp No, we only close "Champion" issues that we decide not to do. This one is assigned the 7.3 milestone because it was completed in C# 7.3.

@ilmax
Copy link

ilmax commented Nov 2, 2018

@gafter should it be marked at completed somehow? Because I feel a bit confusing as well to have open proposal for completed features

@theunrepentantgeek
Copy link

Closed issues are harder to find on GitHub because the issue search includes is:issue is:open by default.

I think the intent is that the tag "C# 7.3" tells you when the feature will be/was released.

@ViIvanov
Copy link
Contributor

ViIvanov commented Nov 3, 2018

@theunrepentantgeek,

Closed issues are harder to find on GitHub because the issue search includes is:issue is:open by default.

Agree

I think the intent is that the tag "C# 7.3" tells you when the feature will be/was released.

Not all knows and can understand, is 7.3 already released or not.

I think, marking as checked "Finalized (done in 7.3)" in start topic is enough for mark issue implemented.

@theunrepentantgeek
Copy link

Not all knows and can understand, is 7.3 already released or not.

For general developers working in C#, I tend to agree with you - the average developer cares more about getting their day to day job done than anything else.

But ...

... here, in the charplang repo, practical discussion of the evolution of C# is the whole reason we're here.

Those who choose tohang out here (even those who just lurk, silently) are, by definition, those who care about such things - and the answer is not hard to find.

@dzmitry-lahoda
Copy link

I use it in release version of C#. Should it be closed?

@yaakov-h
Copy link
Member

yaakov-h commented May 5, 2019

Current policy AFAIK, is that issues remain open until they are included in an updated spec.

@333fred 333fred added the Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification label Oct 16, 2020
@shadow-cs
Copy link

shadow-cs commented Aug 20, 2021

Regarding the recent availability of Generic math preview, it would make sense we also add something along the lines of IEnum which would implement IConvertible<long> (which itself was described as a possible future improvement of generic math in the announcement) as well as I{Span}Parseable (and possibly others) to Enum base type. This would fix further improve this issue in a consistent way.

@333fred
Copy link
Member

333fred commented Aug 20, 2021

This issue was implemented in C# 7.3. If you have a different proposal, please open a new discussion.

@TahirAhmadov
Copy link

@333fred shouldn't this item be closed? I don't mean to dictate anything, just figured it might be one of those things that fell through the cracks.

@HaloFour
Copy link
Contributor

IIRC the issue remains open until the proposal is adopted into the current specification.

@333fred
Copy link
Member

333fred commented Aug 18, 2022

That is correct. The Implemented Needs ECMA Spec label tracks this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion
Projects
None yet
Development

No branches or pull requests