Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Proposal: Nested enum with implicit conversion #587

Closed
lachbaer opened this issue May 16, 2017 · 29 comments
Closed

Proposal: Nested enum with implicit conversion #587

lachbaer opened this issue May 16, 2017 · 29 comments

Comments

@lachbaer
Copy link
Contributor

lachbaer commented May 16, 2017

When there is an enum that has several subsections that could be enums of their own, it would be nice to have those subsections being nested enums.

Now:

    public enum SyntaxKind : ushort
    {
        None = 0,
        List = GreenNode.ListKind,

        // punctuation
        TildeToken = 8193,
        [...]
        // additional xml tokens
        SlashGreaterThanToken = 8232, // xml empty element end
        [...]
        // compound punctuation
        BarBarToken = 8260,
        [...]
        // Keywords
        BoolKeyword = 8304,
        [...]
        // contextual keywords
        YieldKeyword = 8405,
        [...]
        /// when adding a contextual keyword following functions must be adapted:
        /// <see cref="SyntaxFacts.GetContextualKeywordKinds"/>
        /// <see cref="SyntaxFacts.IsContextualKeyword(SyntaxKind)"/>

        // additional preprocessor keywords
        // keywords with an enum value less than ElifKeyword are considered i.a. contextual keywords
        ElifKeyword = 8467,
        [...]
    }

This makes the need for helper functions, that must not be forget to be adapted when a section is extended.

public static IEnumerable<SyntaxKind> GetContextualKeywordKinds()
{
    for (int i = (int)SyntaxKind.YieldKeyword; i <= (int)SyntaxKind.YieldKeyword; i++)
        yield return (SyntaxKind)i;
}

Instead a nesting would be fine, that produces duplicate sub-enums with the same same visibility, underlying type and attributes as the enclosing enum.

    public enum SyntaxKind : ushort
    {
        None = 0,
        List = GreenNode.ListKind,

        enum SyntaxKindPunctuation {
            TildeToken = 8193,
            [...]
        }

        enum SyntaxKindXmlTokens {
            SlashGreaterThanToken = 8232, // xml empty element end
            [...]
        }
        [...]
    }

GetContextualKeywordKinds can now be replaced with the standard
Enum.GetValues(typeof(SyntaxKindPunctuation)) instead of a custom function.

Within the same assembly the nesting inheritance is resolved by the compiler, without further casting.

SyntaxKindPunctuation puncToken = SyntaxKindPunctuation.TildeToken;
SyntaxKind token = puncToken;

With CLR support this implicit casting can be extend to external assemblies as well.

@bondsbw
Copy link

bondsbw commented May 16, 2017

I'd also like a mechanism to include external enums. Something like

// Provided by library
public enum PrimaryColor { Red, Yellow, Blue }
// My code
public enum MyColor
{
    enum PrimaryColor,
    
    enum SecondaryColor
    {
        Orange, 
        Green,
        Purple 
    },

    Amber, 
    Teal,
    Magenta 
}

Then I can write

// Equivalent
MyColor r1 = PrimaryColor.Red;
MyColor r2 = MyColor.Red;

// Equivalent
MyColor o1 = SecondaryColor.Orange;
MyColor o2 = MyColor.Orange;

// As always
MyColor m = MyColor.Magenta;

But these would not be allowed:

PrimaryColor r3 = MyColor.Red;  // ERROR
PrimaryColor o3 = MyColor.Orange;  // ERROR

@bondsbw
Copy link

bondsbw commented May 16, 2017

And I'm not married to allowing MyColor.Red. It wouldn't be able to work when conflicts exist anyway.

@lachbaer
Copy link
Contributor Author

Of course the conversion rules should be exactly as with int and long. From an inner to an outer enum it is an implicit widening conversion and from an outer to an inner enum is an explicit narrowing conversion.

I like the idea of including other enums by name as well. However then the possibilites of duplicate values is high. The programmer must take care of that!

@tannergooding
Copy link
Member

This would make certain enum interop definitions much more clear as well.

@lmcarreiro
Copy link

I did the same request last year when the discussion was in roslyn repository:
dotnet/roslyn#13192

Two months later, @sirgru did to:
dotnet/roslyn#14720

And people are writing ugly code to do something similar:
https://stackoverflow.com/questions/980766/how-do-i-declare-a-nested-enum

But it looks like the C# Language Design Team didn't like it ;(

@CyrusNajmabadi
Copy link
Member

But it looks like the C# Language Design Team didn't like it

There simply isn't a champion for it. That may also mean that no LDM members think this is important enough compared to the current set of work we've picked out.

@lachbaer
Copy link
Contributor Author

this is important enough compared to the current set of work we've picked out

Can we let this quote stand out for its own? 😁 Anyway, this place is a pool of ideas and as long as a topic is still open, it might gafter - ehm, gather - interest for a much later X.X release somewhere in the future.

@CyrusNajmabadi nevertheless it would be nice to have a very little feedback from the LDM team whether this feature is interesting per se or whether it has fallen fallen into disgrace from the very start. :-)

@sirgru
Copy link

sirgru commented May 23, 2017

As mentioned I wanted this functionality a while ago in dotnet/roslyn#14720.
There are 2 problems with enums in my mind as they stand today:

  1. They don't support inheritance. Use case: I have a lot of libraries I reuse across projects. Some of the methods take an enum as argument that must be defined within that library. However, inside the specific project I want to extend the possible values of an enum with meanings sensible in that environment. This is simply not possible as we can't have another enum that inherits the default enum in the library and is supplied as a method argument. The solution I use is making those arguments ints and defining the enum within the project along with a helper extension class for conversion. This is a somewhat clumsy solution from a language point of view, as it loses some "safety". Some people can say that if a method accepts an enum then it could be a series of different methods, but in my case (for example game AI) this logic does not stand. This is similar to what @bondsbw mentioned above.
  2. There is no good mechanism for treating values within the same enum with different grouping. This has traditionally been done with flags, but it's just clumsy. If an enum is a (misnamed) set of expected values, then within a set we should be able to clearly denote subsets and act on them differently. Sometimes the subset can be an element in itself, but it doesn't have to be.

From what I've seen people are against suggestions of this nature as they don't see the benefit in having them (judged by the amount of downvotes on all similar proposals).

@lmcarreiro
Copy link

From what I've seen people are against suggestions of this nature as they don't see the benefit in having them (judged by the amount of downvotes on all similar proposals).

I would like to hear the reasons why people doesn't like it, if it is because it isn't important enough or if they think that this would lead people to write worst code...

@bondsbw
Copy link

bondsbw commented May 23, 2017

@sirgru

They don't support inheritance. Use case: I have a lot of libraries I reuse across projects. Some of the methods take an enum as argument that must be defined within that library. However, inside the specific project I want to extend the possible values of an enum with meanings sensible in that environment.

Passing in an enum with additional values would break the contract of an existing API. You could provide unexpected values, which would generally have undefined behavior.

EDIT: Actually after reading over your comment again, I may be misinterpreting it. If you cannot pass the new values into the existing API, then it shouldn't pose this problem. (I wouldn't call that "inheritance", since inheritance implies substitutability.)

@sirgru
Copy link

sirgru commented May 24, 2017

Well, I don't really want the new values to be able to go into the new API as it can't handle them obviously. Here are a couple of examples of what I would want off the top of my head:

// Library
public enum EventType {
   Event0, Event1
}
public abstract class MyBase {
  public virtual void SomeGeneralEventProcessor (BlaBla argument, EventType eventType) {
    // Does something different depending on the event type
  }
  public abstract void MainDecision(List<EventType> aggregatedEvents);
}
// Project
public enum ExtendedEventTypes : EventType {     // We have new, project-specific event types in addition to pre-defined events.
  Event2, Event3, Event4                         // The allowed range of values is Event0, Event1, Event2, Event3, Event4   
}
public class MyEventWorkingClass : MyBase {      
  // Be able to override the abstract method 
  public override void MainDecision(List<ExtendedEventTypes> aggregatedEvents) {  // Still be able to do the override with the extended enum; either that or allow substitutability and have the possibility of unhandled behavior when supplied in the library.
     // Do the actual work
     foreach(var event in aggregatedEvents) {
       if(event is EventType ) {             // meaning it's either Event0 or Event1
            base.SomeGeneralEventProcessor (BlaBla argument, (EventType) event);
      } else {
            LocalEventProcessor (BlaBla argument, event);
      }
    }
  }
}

So, it's the ability to have new enum values and be able to extend behaviors depending on them.
Here is another one:

  // Library
  public enum MessageTypeBase {
      None
   }
   public interface IMessageable {
        void ReceiveMessage(Message message);
    }
    public class Message {
        public IMessageable     from;
        public IMessageable     to;
        public MessageTypeBase type;
        public float            delay;
        public System.Object[]  data;

        public Message(IMessageable from, IMessageable to, MessageTypeBase type, float delay = 0, params System.Object[] data) {
            this.from = from;
            this.to = to;
            this.type = type;
            this.delay = delay;
            this.data = data;
        }
    }

  // Project
  public enum MessageType : MessageTypeBase {
   Message1, Message2, Message3
  }
  // ...
  MessageQueue.AddMessage(new Message(this, other, MessageType.Message2)); // requires treating the MessageType enum polymorphically.

Edit: I guess in order to avoid the the enum inheritance problem, we could have the partial enum. Such enums would automatically behave polymorphically, while the normal enums wouldn't. In this sense, it is not the breaking of contract as the definition of the partial enum implies more values can be added later.

@lachbaer
Copy link
Contributor Author

public MessageTypeBase type;

It's very dangerous to have values from 'MessageType', that are not part of 'MessageTypeBase', to be assigned to variables of 'MessageTypeBase'! This wouldn't work this way. In this case it would be probably better to create an "enum class" that has full power over inheritance.

class EventType
{
    public const int Event0 = 0;
    public const int Event1 = 1;
}
class ExtendedEventTypes : EventType
{
    public const int Event2 = 2;
    public const int Event3 = 3;
}

It would be worth a proposal to shorten this by typing enum class EventType { Event1 = 1, Event2, Event3 }, what will produce a standard class like above.

public enum ExtendedEventTypes : EventType { Event2, Event3, Event4 }

With what value does 'Event2' start? In this case it seems to be easy, but what if you had this construction:

enum EventType { Event1 = 1 }
enum AdvancedEventType { Event2 = 15, Event3 }
enum ExtremeEventType { Event4 = 3 }

enum SuperduperEventType : EventType, AdvancedEventType, ExtremeEventType { Event5 }

What value will Event5 have? It all gets a bit messy with this. Therefore I stick with the initial idea.

@yaakov-h
Copy link
Member

It's very dangerous to have values from 'MessageType', that are not part of 'MessageTypeBase', to be assigned to variables of 'MessageTypeBase'!

Why? How is this any different from:

enum MyEnum
{
    First,
    Second,
    Third
}

void Foo(MyEnum value) { ... }

Foo((MyEnum)9999);

@lachbaer
Copy link
Contributor Author

@yaakov-h

How is this any different from ... Foo((MyEnum)9999);

1st: the cast, 2nd: you must explicitly cast and thus think about what you're doing. The other example implicitly casts and assigns invalid values without explicitly noticing.

@sirgru
Copy link

sirgru commented May 24, 2017

I don't think the enum class would contribute to safety over something like partial enum. The unsafe part would be an unexpected value, which would remain unhandled by the original library (or manually throw an exception). Something like partial enum, virtual enum or enum interface could establish the expectation that there are more values to be added.

  // Lib
  partial enum MessageType {
      None
  }
 // local
  partial enum MessageType {
      Message1, Message2, Message3
  }

@lachbaer
Copy link
Contributor Author

@sigru partial enum without any inheritance is interesting :-) I would make an addition to it by making an initial value mandatory for every further partial enum, otherwise wich one is the first (with a starting value of 0) wich one is second, aso. Also it should be possible to a pseudonym to that partial, to reference only this subset of values.

partial enum MessageType {
    None    // this has value '0'
}
partial enum MessageType SubMessageType {
    Message1 = 1, 
    Message2, Message3
}

This explicitly makes 'SubMessageType' partial on its own.

@sirgru
Copy link

sirgru commented May 24, 2017

This is why I originally went the inheritance way, it is significant which enum is first and which comes after that, otherwise there can be problems with serialization among toehr things. Also, that's why I had the if(event is EventType ) { // meaning it's either Event0 or Event1 it can be significant whihc subtype the enum is.

The first value being mandatory can actually be problematic. If the original adds another enum value, then that can mean a breaking change for clients. Worse, if clients have serialized values of the enum of the previous int saved to disk, and they change the initial int value to now compile with the new library, the serialized values now mean something else.

@bondsbw
Copy link

bondsbw commented May 24, 2017

Is there something problematic with the extension syntax I described above, which is modeled on the syntax of this proposal?

i.e.

// Library
public enum EventType
{
    Event0,
    Event1
}

// Project
public enum ExtendedEventTypes
{
    enum EventType,

    Event2,
    Event3,
    Event4  
}

The issue of inheritance has been discussed before, which is why I purposely avoided inheritance syntax and all the issues that come as a result.

@sirgru
Copy link

sirgru commented May 24, 2017

Well, it wouldn't work in my cases, as ExtendedEventTypes is effectively completely separate from EventType, and the method overrides wouldn't work.

@lachbaer
Copy link
Contributor Author

@bondsbw partial enums had the adavantage that they can be defined in seperate code files without any need for a further, unifiying enum.

@bondsbw
Copy link

bondsbw commented May 24, 2017

@sirgru In your first example, you are overriding a method by changing its signature (in your case it changes the argument type List<EventType> to List<ExtendedEventTypes>). That is disallowed for the same reason of substitutability; any code that relies on MyBase could break if MyEventWorkingClass is substituted and one of the new enum values is supplied to the method call.

Hiding the method or using a different method name would not break inheritance. If you need the new method to call the base method, then you will need to work out how to properly convert any extended enum values into the original enum type.


EDIT: Actually I'm not sure this really breaks things. The new enum isn't "inheriting" the original (a subset type). It is a superset type. It would be akin to the following:

public class A
{
    public virtual void Foo(string s) { }
}
public class B : A
{
    public override void Foo(object o) { } // object is a superset type of string
}

It isn't currently legal to make a contravariant change to a parameter type in a derived class, but I'm not sure it would actually violate type safety. It might create more confusion than it's worth, however.

@bondsbw
Copy link

bondsbw commented May 24, 2017

@lachbaer I'm not against partial enums, so long as all parts are defined within the same assembly.

But I don't see how SubMessageType is actually partial. It has a different name, therefore the second part can't be freely substituted for the first. It's like the definitions class Animal and partial class Dog : Animal; there's no reason for the partial modifier.

@lachbaer
Copy link
Contributor Author

@bondsbw SubMessageType is only a synonym for that subset. I haven't expressed that correctly. SubMessageType is not part of that partial super-enum. It is very similar to the nested enum from the initial proposal.

@bondsbw
Copy link

bondsbw commented May 24, 2017

Ok, in that case it is just confusing. If the initial proposal is implemented, then to me it makes more sense to build on that syntax for extension than to come up with something completely different that puts an alternative definition on existing keywords.

@lachbaer
Copy link
Contributor Author

It can both be combined to take the best of both worlds.

// file1.cs
enum SubMessageType {
    Message2 = 2, 
    Message3
}

// file2.cs
partial enum MessageType {
    None,    // this has value '0'
    Message1 
}

// file3.cs
partial enum MessageType {
    let enum SubMessageType  // `let` = include existing enum instead of creating a new nested one
}

// Enum.GetValues(typeof( SubMessageType )) = 2, 3
// Enum.GetValues(typeof( MessageType )) = 0, 1, 2, 3

@bondsbw
Copy link

bondsbw commented May 24, 2017

So long as those partial definitions are in the same assembly, I think that is reasonable.

Though when we start talking about whether partial enums help more than they cause confusion, I don't know that I can stand behind them. The main use cases for partial classes are code generation and designers, but I don't know that enums would enjoy benefits in those situations. They may be worth a separate proposal.

@lmcarreiro
Copy link

This discussion was forked into "nested enum" and "enum inheritance" and already take the way of enum inheritance.

Maybe it's better to change the title and description of this issue to lead to enum inheritance only, and create a separate proposal for nested enum.

@bondsbw
Copy link

bondsbw commented May 27, 2018

A concern raised at #1569 needs to be addressed by this proposal. Namely, how do the default values get assigned for the wrapper/superset enums, and do they overlap.

@grzfes
Copy link

grzfes commented Jul 29, 2018

So it would have to be something like array used like enum and (auto-filled) extended by class inheritance?
With possibility of having duplicates (aliases), plus covering the entire group in switch.

public enum MyColor
{                          //
    enum PrimaryColor,     //              [0]
                           // {
                           //   Red,       [0][0]
                           //   Yellow,    [0][1]
                           //   Blue       [0][2]
                           // }
    enum SecondaryColor    //              [1]
    {
        Orange,            //              [1][0]
        Green,             //              [1][1]
        Purple             //              [1][2]
    },

    Amber,                 //              [2]
    Teal,                  //              [3]
    Magenta                //              [4]
}

If underlying type of MyColor is Int32, result will be int[] - each dimension value, e.g.:
int {1, 2} for MyColor.SecondaryColor.Purple.

  • Or perhaps 2 for compatibility (think about extending completely filled ulong enum¹)?

Limits:

  • duplicates must be declared in the same enum;
  • none enum can be of a type that can accommodate more than (in this example) int.

Without indexing groups, continue numbering?

So in this case:

// Provided by library
public enum PrimaryColor { Red, Yellow, Blue }
public enum MyColor
{
    enum PrimaryColor,
    
    enum SecondaryColor
    {
        Orange, 
        Green,
        Purple 
    },

    Amber, 
    Teal,
    Magenta 
}
  • which index will be assigned to Orange and Amber by default?
    • what will the indexes be, if you want to extend two enum?
      • not allowed²,
      • the same as declared in each enum,
        • how to distinguish MyColor.Secondary.Orange from MyColor.Red?
      • next available after the last value,
      • each subsequent gap from 0
        • how to treat negative indices?
  • what will happen if PrimaryColor will be changed (added values)?
    • (even if noticed) how to remedy it?
      • check if new values was passed as old enum?

Not array as result?

  • what if someone want to extend ulong enum¹, and all of it values are already used?

¹ - ulong is underlying type of enum, please accept that this is possible (not necessary real case).
² - If there are to be any limitations in how enum can be extented, such as:

  • only from one,
  • not from something like sealed enum,
  • not from other assemblies etc.

then it is at best a serum, not a cure.
And to think this possibly will be breaking change...

partial enum

Look for all used values in many files, which will not necessarily be in one folder, and even if... ?

OK, if all inherited enums:

  • will have their own numbering, which will not affect the whole;
  • only as entire they will be divided into files.

@dotnet dotnet locked and limited conversation to collaborators Dec 3, 2024
@333fred 333fred converted this issue into discussion #8727 Dec 3, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants