Skip to main content
GameDev.net gamedev.net
🔒 Locked

Why are enums broken

Started by TheComet Jun 9, 2018 at 10:51 PM 28 replies 7.6k views
Original Post
TheComet
TheComet

enum class Thing {
    FOO = 0x01, 
    BAR = 0x02
};

Thing thing = Thing::FOO | Thing::BAR; // doesn't compile

This is an age old problem. What is the proper way to achieve what I'm trying to do here? As far as I can tell, using enum class is more a hindrance than a useful tool with flags. I end up typing less with a normal enum:


enum Thing {
    FOO = 0x01, 
    BAR = 0x02
};

unsigned char thing = FOO | BAR;


"I would try to find halo source code by bungie best fps engine ever created, u see why call of duty loses speed due to its detail." -- GettingNifty
dawgdog
dawgdog

This is the right behavior for an enum class. Normal enums are integers and can be used in the same way as integers, but this can cause problems. Imagine you have an enum that holds read/write flags for file I/O. With regular enums you can do something like READ + 5, but what does that mean? How can you add 5 to something that represents the concept of opening a file for reading?

Enum classes get around this issue by disallowing most operators by default. You have to overload the right operators for your use cases. For your case you need to overload operator|.

Oberon_Command
Oberon_Command

You could also investigate using std::bitset to represent flags instead of manually or/anding the bits. You can still use an enum for this, but then each enum value would just be a name for a bit.

Alberth
Alberth

enum classes enforce that you cannot construct new values from them, unless you cast. The primary reason is to make it explicitly visible in the source code that you're using enum names as integer values rather than as a symbolic name (where the value is not very relevant). In other words, it eliminates the gap of using enum values as hidden constants.

C++ being C++, it's of course not impossible, you can add casts, and hide such things in an overloaded operator.

The point is however that you should consider whether your enum class is really an enum class, or that a set of (integer) constants would be more fitting.

a light breeze
a light breeze

I would not use an enum at all here, class or otherwise. The quick and dirty way to do this is to just use constants:


namespace Thing {
  constexpr uint8_t FOO = 0x01;
  constexpr uint8_t BAR = 0x02;
}

The fancy way is to define a full (non-enum) class that implements operator |, operator &, and ideally an operator <<(std::ostream&, Thing) that provides a human-readable representation (e.g. "FOO|BAR" instead of "3").

Promit
Promit
13 hours ago, a light breeze said:

I would not use an enum at all here, class or otherwise. The quick and dirty way to do this is to just use constants:



namespace Thing {
  constexpr uint8_t FOO = 0x01;
  constexpr uint8_t BAR = 0x02;
}

The fancy way is to define a full (non-enum) class that implements operator |, operator &, and ideally an operator <<(std::ostream&, Thing) that provides a human-readable representation (e.g. "FOO|BAR" instead of "3").

The problem with this approach is that it doesn't create a type or type alias that you can use to cue in what values are actually expected for a given parameter or variable. If I have a function, I can say that the type of one argument is Thing and then i know that Thing::FOO and Thing::BAR are valid options. Can't do that if Thing is a namespace.

I get around this with some magic macros that create a struct named Thing with an int cast, and create the enum values inside that struct.

SlimDX | Ventspace Blog | Twitter | Diverse teams make better games. I am currently hiring capable C++ engine developers in Baltimore, MD.
ChaosEngine
ChaosEngine
1 hour ago, Promit said:

The problem with this approach is that it doesn't create a type or type alias that you can use to cue in what values are actually expected for a given parameter or variable. If I have a function, I can say that the type of one argument is Thing and then i know that Thing::FOO and Thing::BAR are valid options. Can't do that if Thing is a namespace.

I get around this with some magic macros that create a struct named Thing with an int cast, and create the enum values inside that struct.

You could use BOOST_STRONG_TYPEDEF or something similar, but I really wish that strong or opaque typedefs were part of the language.



if you think programming is like sex, you probably haven't done much of either.-------------- - capn_midnight
frob
frob
On 6/9/2018 at 5:51 PM, TheComet said:

As far as I can tell, using enum class is more a hindrance than a useful tool with flags.

That's correct. The purpose of strongly typed enums was to allow strict enforcement of enumerations. It prevents things like (Blue+5) which may not be a valid enumeration value, or (Walking | Yellow) which may not be a valid combination. You can list all the states in your FSA and be sure you're not getting unexpected values that aren't enumerated at all.

Flag operations are directly contrary to the strong type since they're asking to be treated as bit patterns rather than enumerations. That is, you are not using them like a strongly typed enumeration where you're only dealing with the items on the list, instead you're treating them as a pattern of bits.


One pattern that can help keep them within a name is to keep them as weakly typed values inside a namespace:


namespace MyFlags {
  enum MyFlags : int {
    none = 0,
    foo = 1<<0,
    bar = 1<<1,
    baz = 1<<2,
    thing1 = 1<<3,
    thing2 = 1<<4,
    ...
  };
}


Alternatively you can use a mix of templates and/or macros to generate the expected operations for your flags. There are plenty of examples of it online that compile away to constant expressions that are optimized back down to bitfield operations, my first search pulls up four different github projects for it and a bunch of stackoverflow and similar postings.

Just remember that you aren't using the feature the way it was designed.

If you want bit manipulation then use bit manipulation. If you want a strong enumeration where you're only dealing with the enumerated values, use an enum class. They're different use cases and they have different meanings and different requirements.

dragongame
dragongame

struct MyBitField {
  unsigned int foo : 1;
  unsigned int bar : 1;
  ...
};

You could also do bit fields. They can tell you exactly your intend. Just another idea.

“Always programm as if the person who will be maintaining your program is a violent psychopath that knows where you live”
frob
frob
3 hours ago, dragongame said:

You could also do bit fields.

You can do that if you don't care about the specific bits involved. The language standard says the allocation order for the bits is implementation defined. You don't know which bits are assigned to which positions so everything needs to be done by name.

Different ABIs have different bit ordering. If you're using the x64 ABI it is packed like this, and you'll note it can include spaces between items. Other platform ABIs have different packing requirements, and it looks like GCC supports six variants depending on the target platform.


True they are still bit flags, but beware they may not be the human-intuitive values that other programmers expect.

ChaosEngine
ChaosEngine
2 hours ago, frob said:

You can do that if you don't care about the specific bits involved. The language standard says the allocation order for the bits is implementation defined. You don't know which bits are assigned to which positions so everything needs to be done by name.

Different ABIs have different bit ordering. If you're using the x64 ABI it is packed like this, and you'll note it can include spaces between items. Other platform ABIs have different packing requirements, and it looks like GCC supports six variants depending on the target platform.


True they are still bit flags, but beware they may not be the human-intuitive values that other programmers expect.

Would std bitset be a better choice?

if you think programming is like sex, you probably haven't done much of either.-------------- - capn_midnight
Finalspace
Finalspace

The main problem is: Enums are often misused as flags and even i use it that way, because i want my flags to be grouped.

A group of flags which can be validated is much safer and easier to use than random constants with a special naming convention.

Randoms constants defined in a namespace is not the same as grouped flags - you wont get compile error validation!


My way of doing it is the following:



#define ENUM_AS_FLAGS_OPERATORS(etype) \
	inline etype operator | (etype a, etype b) { \
		return static_cast<etype>(static_cast<int>(a) | static_cast<int>(b)); \
	} \
	inline bool operator & (etype a, etype b) { \
		return (static_cast<etype>(static_cast<int>(a) & static_cast<int>(b))) == b; \
	} \
	inline etype& operator |= (etype &a, etype b) { \
		return a = a | b; \
	}

enum class MyEnumAsFlags : int {
  None = 0,
  A = 1 << 0,
  B = 1 << 1,
  C = 1 << 2,
};
ENUM_AS_FLAGS_OPERATORS(MyEnumAsFlags);


But this is a stupid and should not be required - really there should be a something like this (without requiring any templated whatsoever, it should just be part of the language):



// For flags just make it clear its a enumeration of flags (First entry is 1, second is 2, third is 4, etc.)
// its so simple - dont understand why the c++ standard does not contain this
enum flags MyFlags {
  A, // Is 1
  B, // Is 2
  C, // Is 4
};
enum flags MyFlags {
  None = 0,
  A = 1,
  B = 2
  C, // C is 4
};

MyFlags w = MyFlags::A | MyFlags::B;
MyFlags t = 0x3; // Compile error
MyFlags f = 0x2; // Translated to MyFlags::B
MyFlags d = 0; // No flags -> Totally valid to pass zero here

// flags with fixed integral type
enum flags MyByteFlags : int8_t {
  A = 0x1,
  B = 0x2
};

// enum class without type
enum class MyChoice {
  None,
  One,
  Two
};

MyChoice w = MyChoice::One;
MyChoice t = MyChoice::One | MyChoice::Two; // Compile error
MyChoice f = 1; // Compile error

// enum class with fixed integral type (First entry is one always, unless its overriden)
enum class MyChoice : int32_t {
  None = 0,
  One,
  Two
};

MyChoice w = MyChoice::One;
MyChoice t = MyChoice::One | MyChoice::Two; // Compile error
MyChoice f = 3; // Compile error
MyChoice d = 1; // Translated to MyChoice::One


frob
frob
On 6/12/2018 at 3:23 AM, Finalspace said:

it should just be part of the language

It is, and it doesn't break anything: Use a non-class version of an enum.

There are two different needs here.

The use of strongly typed enumerated values as exactly that: enumerated values. You have a set of values and they only make sense as the values themselves. An enumeration like { Cheddar, Mozzarella, Roquefort, Provolone, Monterrey Jack, Pepper Jack, Feta } is an example. A n enumeration like { Fork, Knife, Spoon, Cup, Plate } is another. The bitwise operations on actual enumerations does not make sense. What does it mean to take the bitwise AND of Fork and Spoon? What does it mean to take the bitwise OR of Cheddar and Pepper Jack? I can understand a logical operations, but not bitwise operations.

A flag means using an enum as a named integer value representing a bit pattern. They can be manipulated as flags but they are no longer enumeration values when you do so. When the integer value is a single bit or pattern of bit, then bitwise operations make sense. The bitwise AND of Bit3 and Bit7 is something meaningful. Unless someone has made the effort to enumerate every one of the 2^n combinations, it probably is not specifically enumerated.


The .net framework -- note that it is not the language itself but a core library -- has their [Flags] attribute, but people have done exactly the same thing with templates and macros in C++. In C# you create your enum and write the line [Flags] on the line above it. In C++ you create your enum and write the line ENUM_AS_FLAGS (or whatever your code library calls it) on the line below it. In both cases the cost to the programmer is a single indicator. In both cases it is not part of the language itself, but instead part of libraries people use.


Oberon_Command
Oberon_Command
On 6/12/2018 at 1:23 AM, Finalspace said:

A group of flags which can be validated is much safer and easier to use than random constants with a special naming convention.

If you need flags, why would you not just use std::bitset? This kind of thing is exactly what it's meant for. You can still put an enum into a std::bitset and eliminate the "abuse", albeit with some casting. If that bothers you, you can always write a template wrapper for std::bitset that specializes on the enum class and does the casting for you.

alvaro
alvaro

I find myself wanting to do arithmetic operations and bitwise operations on enums all the time. Here's an example from my chess engine:


enum Piece {
  Pawn, WhitePawn = Pawn, BlackPawn,
  Knight, WhiteKnight = Knight, BlackKnight,
  Bishop, WhiteBishop = Bishop, BlackBishop,
  Rook, WhiteRook = Rook, BlackRook,
  Queen, WhiteQueen = Queen, BlackQueen,
  King, WhiteKing = King, BlackKing,
  Empty, NPieces = Empty
};

I use this type as index in an array declared something like uint64_t bb[NPieces]'. This is a bitboard representation of the position, where bb[WhitePawn] is the 64-bit integer whose bits indicate the presence or absence of a white pawn at each location. Then I can refer to the pawns of the side to move as bb[Pawn + side_to_move]', where `side_to_move' is 0 for white and 1 for black (I have an enum for that too). The result of that sum is an int [I believe] and if I wanted to store it in a variable of type Piece I would have to cast it.

Another operation that I may want to do is loop over the piece types, which also requires arithmetic.


  for (Piece piece = WhitePawn; piece != NPieces; piece = Piece(piece + 1))

It would be nice if I didn't have to fight the compiler to accept ++piece' as my increment.

In order to check if a piece is a pawn of either color, I can do ((piece & ~1) == Pawn)'.

I don't have much of a problem working within the language: I either define some operators to return Piece or I sometimes use int' in places where Piece' would be more expressive, so the compiler doesn't complain. But I can see how some people might feel that enums are too restrictive.


ChaosEngine
ChaosEngine

@alvaro I feel like if you're going to do that kind of code, Piece should really be a class where you can overload the ++ operator.

Then you can do Piece.IsPawn etc. In theory, this shouldn't cost you anything in terms of performance, but I'll grant that a chess engine is not something where you have a lot of margin for extra space, etc.

if you think programming is like sex, you probably haven't done much of either.-------------- - capn_midnight
Finalspace
Finalspace
12 hours ago, Oberon_Command said:

If you need flags, why would you not just use std::bitset? This kind of thing is exactly what it's meant for. You can still put an enum into a std::bitset and eliminate the "abuse", albeit with some casting. If that bothers you, you can always write a template wrapper for std::bitset that specializes on the enum class and does the casting for you.

No i dont want to use bitset or anything else from the STL - just to get grouped flags. I think its just stupid that its not part of the language itself. Object pascal has it since ages, set of and other useful features: http://www.delphibasics.co.uk/Article.asp?Name=Sets

12 hours ago, frob said:

It is, and it doesn't break anything: Use a non-class version of an enum.

Enums until C++/11 are pretty much useless in my opponion. Why? All enum values are in the global namespace + Weak compile time validation.

alvaro
alvaro
4 hours ago, Finalspace said:

Enums until C++/11 are pretty much useless in my opponion. Why? All enum values are in the global namespace + Weak compile time validation.

The enum values before C++11 were in whatever namespace you defined the enum, not necessarily in the global namespace.


Finalspace
Finalspace
54 minutes ago, alvaro said:

The enum values before C++11 were in whatever namespace you defined the enum, not necessarily in the global namespace.


True, but you can access each member inside it without specify the enum at all, which makes each value pretty much a constant. Example:


namespace abc {
	enum OldCPPEnum {
		A,
		B,
		C
	};

	static void OldCppEnumTest() {
		OldCPPEnum x = OldCPPEnum::B;
		OldCPPEnum y = A;
	}
}
namespace cba {
	static void OldCppEnumTest() {
		abc::OldCPPEnum x = abc::B;
		abc::OldCPPEnum y = abc::OldCPPEnum::A;
	}
}


alvaro
alvaro

I like your edit from "Wrong" to "True". :)



Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.