Jump to content
  • Advertisement
Finalspace

C++ Problem with enum and binary or operator

This topic is 391 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Why wont this code compile? How do i get this to compile (VC++ 2015)?

typedef enum fpl_InitFlag {
	fpl_InitFlag_None = 0,
	fpl_InitFlag_Window = 1 << 0,
	fpl_InitFlag_VideoOpenGL = 1 << 1,
} fpl_InitFlag;

static void InitSomething(fpl_InitFlag initFlags) {
	if (initFlags & fpl_InitFlag_VideoOpenGL) {
		initFlags |= fpl_InitFlag_Window;
	}
}
int main(int argc, char **args) {
	InitSomething(fpl_InitFlag_VideoOpenGL);
	return 0;
}

Error C2676    binary '|=': 'fpl_InitFlag' does not define this operator or a conversion to a type acceptable to the predefined operator

Error (active)        this operation on an enumerated type requires an applicable user-defined operator function

Edited by Finalspace

Share this post


Link to post
Share on other sites
Advertisement

Found a solution for C++ - not great that but will work:

inline fpl_InitFlag operator |(fpl_InitFlag a, fpl_InitFlag b) {
	return static_cast<fpl_InitFlag>(static_cast<int>(a) | static_cast<int>(b));
}
inline fpl_InitFlag operator &(fpl_InitFlag a, fpl_InitFlag b) {
	return static_cast<fpl_InitFlag>(static_cast<int>(a) & static_cast<int>(b));
}
inline fpl_InitFlag& operator |=(fpl_InitFlag& a, fpl_InitFlag b) {
	return a = a | b;
}

 

Edited by Finalspace

Share this post


Link to post
Share on other sites

I think using just enum and not enum class is the better option for bitfields, sadly. But i'm curious what comes up here... :)

Share this post


Link to post
Share on other sites

You can read this.

 

I personally don't like to do that just because doing arithmetic (including bitwise operations) on enums definitely lead to have values not defined in the enum. So you start with a finite set of elements and by allowing such operations you can end up with an infinite set of elements (if you limit to 'or' bitwise operations you are still stuck with a finite number of elements, but their number is large).

So typical C operations like a switch will not be able to handle easily all the values. Also, when debugging, the debugger will not be able to print the matching name of an enum value.

And if you are in C++, this tends to pervade the nature of your enumeration type.

This is just what I think about that :)

Share this post


Link to post
Share on other sites

The compiler is just trying to save you from writing buggy code.  You need to go out of your way to force your bugs into your software.  Your solution does a fine job of doing that.

You're implementing an operator that is not closed on the enumeration set.  You're breaking the contract provided by enum.  When you start getting weird errors, save yourself some debugging time and check where you use the enum first.

Share this post


Link to post
Share on other sites
51 minutes ago, Bregma said:

The compiler is just trying to save you from writing buggy code.  You need to go out of your way to force your bugs into your software.  Your solution does a fine job of doing that.

You're implementing an operator that is not closed on the enumeration set.  You're breaking the contract provided by enum.  When you start getting weird errors, save yourself some debugging time and check where you use the enum first.

A normal enum are just a single 32 bit integer value, including its field accessible by a constant.

You can even define which type your enum are. So using it as flags are totally valid, because its just a stupid integer.

And for the case the compiler may change it to maybe 64 bit integer, i dont care - because the binary operators works there as well.

 

There is no buggy code at all - its totally fine. I see no side-effects whatsoever - unless i overwrite the operators doing some weird shit.

 

Sure i could change it to enum class, but this will break C compability entirely and for a C library this is no go!

Edited by Finalspace

Share this post


Link to post
Share on other sites

I've found the following pattern to be effective:

#include <stdio.h>
  
constexpr const unsigned flag[]                                                                                                                                    
= {                                                                                                                                                                
	0, 1, 2, 4, 8, 16, 32, 64, 128, 256                                                                                                                            
};                                                                                                                                                                 
                                                                                                                                                                   
struct fpl                                                                                                                                                         
{                                                                                                                                                                  
	enum init_flag                                                                                                                                                 
	{                                                                                                                                                              
		init_none,                                                                                                                                                 
		init_window,                                                                                                                                               
		init_opengl                                                                                                                                          
	};                                                                                                                                                             
                                                                                                                                                                   
	fpl( const unsigned &iflags )                                                                                                                                  
	: init_flags{ iflags }                                                                                                                                         
	{}                                                                                                                                                             
                                                                                                                                                                   
	unsigned init_flags;                                                                                                                                           
};                                                                                                                                                                 
                                                                                                                                                                   
                                                                                                                                                                   
int main( int argc, const char *args[] )                                                                                                                           
{                                                                                                                                                                  
	fpl my_fpl{ flag[ fpl::init_window ] | flag[ fpl::init_opengl ] };                                                                                       
                                                                                                                                                                   
	printf( "my_fpl flags: 0x%X\n", my_fpl.init_flags );                                                                                                           
                                                                                                                                                                   
	return 0;                                                                                                                                                      
}                                                                                                                                                                  

 

Share this post


Link to post
Share on other sites

I think using enumerations as flags is fine.  The whole argument that 'its bad because they're not supposed to be used that way' I think is kinda silly.  Sure you don't want to have a situation where you accidentally create an undefined bit pattern, but whether that bit pattern is an 'enum' or just an uint32_t, you still have the same error.  It'll be the same problem in the same piece of code.  The nice thing about enum's is you can have nicer names and avoid stuff like VK_STRUCTURE_TYPE_IMPORT_MEMORY_WIN32_HANDLE_INFO_NV.

Here is what I use (formatting is a bit off but you get the idea):

// --------------------------------------------------------------------------------------------------------------------------
//	enumeration expansion
//		- ENUM_CLASS_OPERATORS defines standard bit operators for enum class types
//		- ENUM_CLASS_AND_OR defines only 'and' and 'or'
// --------------------------------------------------------------------------------------------------------------------------

# define ENUM_CLASS_OPERATORS(T)																																					\
inline constexpr T operator~(T a) noexcept { return static_cast<T>(~static_cast<uint64_t>(a)); }														\
inline constexpr T operator&(T a, T b) noexcept { return static_cast<T>(static_cast<uint64_t>(a) & static_cast<uint64_t>(b)); }			\
inline constexpr T operator|(T a, T b) noexcept { return static_cast<T>(static_cast<uint64_t>(a) | static_cast<uint64_t>(b)); }			\
inline constexpr T operator^(T a, T b) noexcept { return static_cast<T>(static_cast<uint64_t>(a) ^ static_cast<uint64_t>(b)); }			\
inline T& operator&=(T& a, T b) noexcept { return a = static_cast<T>(static_cast<uint64_t>(a) & static_cast<uint64_t>(b)); }			\
inline T& operator|=(T& a, T b) noexcept { return a = static_cast<T>(static_cast<uint64_t>(a) | static_cast<uint64_t>(b)); }				\
inline T& operator^=(T& a, T b) noexcept { return a = static_cast<T>(static_cast<uint64_t>(a) ^ static_cast<uint64_t>(b)); }

# define ENUM_CLASS_AND_OR(T)																																						\
inline constexpr T operator&(T a, T b) noexcept { return static_cast<T>(static_cast<uint64_t>(a) & static_cast<uint64_t>(b)); }			\
inline constexpr T operator|(T a, T b) noexcept { return static_cast<T>(static_cast<uint64_t>(a) | static_cast<uint64_t>(b)); }			\
inline T& operator&=(T& a, T b) noexcept { return a = static_cast<T>(static_cast<uint64_t>(a) & static_cast<uint64_t>(b)); }			\
inline T& operator|=(T& a, T b) noexcept { return a = static_cast<T>(static_cast<uint64_t>(a) | static_cast<uint64_t>(b)); }	

The thing to consider is, even with bit operators, its actually quite hard to come up with a bit pattern that's undefined.  Most of code with flags looks something like:

enum class EEnumOptions {
  none,
  option_1,
  option_2,
  option_3,
  };

enum class EEnumFlags {
  none = 0,
  flag_a = 1,
  flag_b = 2,
  flag_c = 4,
  };
ENUM_CLASS_AND_OR(EEnumFlags)	// create 'and' and 'or' bit operators for EEnumName
  
// ....

void Func(EEnumOptions e, EEnumFlags f) {
  
  // handle options
  switch (e) {
    case EEnumOptions::option_1:
    case EEnumOptions::option_2:
    case EEnumOptions::option_3:
    }
  
  // handle flags
  if ((f & EEnumName::flag_a) == EEnumName::flag_a) {}		// flag_a is set
  if ((f & EEnumName::flag_b) == EEnumName::flag_b) {}		// flag_b is set
  if ((f & EEnumName::flag_c) != EEnumName::flag_c) {}		// flag_c is not set
  }

Even if you were to make a silly bit pattern, things won't 'blow up'.  Any bit pattern is still well defined.  Also if you only restrict yourself to 'and' and 'or' (ie. don't overload 'not' and 'xor'), then its near impossible to create undefined bit patterns (short of intentionally static_cast'ing them in).  Its still safer then simply integer constants or #define's, and for the most part self documenting.  I don't think anyone would have any difficulty using that function, or understanding what is expected, and passing an undefined bit pattern would have to be intentional.

Maybe its my own personal preference, but this seems clean, easy to understand, and hard to break; and isn't that what we want?

Share this post


Link to post
Share on other sites

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!