Problem with enum and binary or operator

Started by
51 comments, last by ChaosEngine 6 years, 9 months ago

This sucks really. I added another enum overload:

 


typedef enum fpl_KeyboardModifierType {
	fpl_KeyboardModifierType_None = 0,
	fpl_KeyboardModifierType_Alt = 1 << 0,
	fpl_KeyboardModifierType_Ctrl = 1 << 1,
	fpl_KeyboardModifierType_Shift = 1 << 2,
	fpl_KeyboardModifierType_Super = 1 << 3,
} fpl_KeyboardModifierType;

#ifdef __cplusplus
	inline fpl_KeyboardModifierType operator |(fpl_KeyboardModifierType a, fpl_KeyboardModifierType b) {
		return static_cast<fpl_KeyboardModifierType>(static_cast<int>(a) | static_cast<int>(b));
	}
	inline fpl_KeyboardModifierType operator &(fpl_KeyboardModifierType a, fpl_KeyboardModifierType b) {
		return static_cast<fpl_KeyboardModifierType>(static_cast<int>(a) & static_cast<int>(b));
	}
	inline fpl_KeyboardModifierType& operator |=(fpl_KeyboardModifierType& a, fpl_KeyboardModifierType b) {
		return a = a | b;
	}
#endif

and now i get this:

 

Error    C2733    'operator |': second C linkage of overloaded function not allowed

Error    C2733    'operator &': second C linkage of overloaded function not allowed

Error    C2733    'operator |=': second C linkage of overloaded function not allowed

 

I am nearly at a point, where i just want to throw of all enums and use just a struct with an uint32 value and use simple defines - so i get type checking at least for the argument type...

 

People who saying "Almost all legacy C code can be compiled with a C++ compiler" are just lying, because even this simple thing wont compile -.- I see why other C libraries dont use enums at all...

Advertisement
17 hours ago, Ryan_001 said:

The whole argument that 'its bad because they're not supposed to be used that way' I think is kinda silly.

I think breaking contracts for no good reason while better alternatives exist is kinda malicious. Most, if not all bugs, originate from false assumptions. Assuming an enumeration type can only hold a single value from the enumeration has turned into a falsehood by shoehorning it into something it's not.

18 hours ago, Ryan_001 said:

Sure you don't want to have a situation where you accidentally create an undefined bit pattern

What would that be? I'd say that a randomly generated number is still well defined, a 0-bit means a flag is unset, a 1-bit means it's set. If you only have two flags, it doesn't really matter what the 3rd or 4th bits are set to. If you're talking about certain bits being mutually exclusive, your proposed solution does nothing to prevent that. It's also very common to use the bitwise not operator to disable flags, it's easy to understand and recognize.

What's this safety you are talking about? As far as I can tell you have gained nothing in preventing a programmer from making mistakes. Not only that, you've decreased legibility and increased the mental load to deal with silly mistakes. Mistakes that should barely take any time at all to fix in the first place, for some misguided sense of safety.

18 hours ago, Ryan_001 said:

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.

The difference is that an enum is explicitly designed to stop you getting into the situation where you're using unexpected values, by asking you to specify which value you want by name. Deliberately thwarting that system means you lose some of the benefits. (You keep some benefits, such as a degree of type safety, so I can see why it's frustrating to have an all-or-nothing decision here.)

Going back to the original post and the most recent problem:

6 minutes ago, Finalspace said:

Error    C2733    'operator |': second C linkage of overloaded function not allowed

Looks like the function is not inlined. I've no idea why that might be the case, but maybe there are some settings or macros that switch it off.

44 minutes ago, Kylotan said:

The difference is that an enum is explicitly designed to stop you getting into the situation where you're using unexpected values, by asking you to specify which value you want by name. Deliberately thwarting that system means you lose some of the benefits. (You keep some benefits, such as a degree of type safety, so I can see why it's frustrating to have an all-or-nothing decision here.)

Going back to the original post and the most recent problem:

Looks like the function is not inlined. I've no idea why that might be the case, but maybe there are some settings or macros that switch it off.

For me enums are just a group of named integers, so i can access it in C++ by Enum::A and in C Enum_A.

How i use them, is up to me - maybe i just want flags, maybe i want single states? I dont care.

 

If i want extra hard type safety, i just use enum class - but most of the time i just want grouped flags, so i dont accidently set a keyboard_flag on a init_flag... Thats all i want for safety.

Is that so hard for c++? Other languages like c# and even old pascal can do this right.

 

Hmm i know that inline is just a "hint" to the compiler, but this is totally wrong... maybe i need some template magic to get it to compile and behave.

5 minutes ago, Finalspace said:

For me enums are just a group of named integers [...]

How i use them, is up to me - maybe i just want flags

But the key point there, is that your combination of 2 enum values boolean-ored together is no longer in your "group of named integers". It's related to them, sure, but it's outside the group. It was never named, after all.

Imagine this:


const int ONE = 1;
const int TWO = 2;

And now say you want to be able to perform division on 2 of these values, which returns 0.5. But 0.5 isn't in the 'int' set, so you probably shouldn't expect to be able to pretend that it is the same type, just like the value corresponding to 3 isn't in fpl_InitFlag. C and C++ let you get away with it for enums because of an implementation detail that they're stuck supporting forever.

Arguably there should be a different concept entirely for boolean flags like this, but we don't have that in C++.

 

11 minutes ago, Finalspace said:

Is that so hard for c++?

It's not, and it works in C++, and many people have been exploiting this equivalence to integers for a long time. You're almost certainly just doing something wrong at your end.

21 hours ago, Ryan_001 said:

The whole argument that 'its bad because they're not supposed to be used that way' I think is kinda silly.

It doesn't matter that it's silly. That's the rules of the language as laid down by the spec. It's the assumptions that your optimizing compiler holds up as golden.

You break the rules of the language and the compiler is allowed to fuck you.
e.g. you can play with pointers willy-nilly, but the optimizer is allowed to assume that you haven't broken the aliasing rules.


float f = 1.0f;
(*(int*)f) = 0; // note that int(0) and float(0) have the same bitpattern, so this should work just fine! Yay, clever!
printf( "%f", f );//does this print 1 or 0?

In practice, that will often print 0.000... but the rules of the language say that this program is invalid, so you could expect it to print 1.000 as well. The aliasing rule says that on the 3rd line it's accessed as a float type, so only the most recent write to a float type could possibly impact the value. The 2nd line can be optimized out or otherwise ignored. That's the specified rules of the language. 

Likewise:


enum Flags {
 F_1 = 1,
 F_2 = 2,
 F_4 = 4,
};

Flags f = (Flags)((int)F_1 | (int)F_2 | (int)F_4);
printf( "%d", (int)f );//does this print 7?

In practice, this will probably print 7... but the rules of the language say that the variable f is only allowed to hold values from 1 to 4 (inclusive), so you could expect this program to print out something entirely different, such as 4 as well.

That might sound silly, but optimizers are built to follow the rules of the language! So, say you've written a switch statement that's designed to handle the 8 possible values that you expect the above 'f' variable to hold, the compiler is well within it's rights to assume that half of these cases are impossible as the spec says that it can only hold values 1-4, and therefore the compiler is safe to go ahead and simply optimize away those function calls completely out of existence.


switch( (int)f )
{
  default: return Case_Other();//impossible
  case 0: return Case0();//impossible
  case 1: return Case1();
  case 2: return Case2();
  case 3: return Case3();
  case 4: return Case4();
  case 5: return Case5();//impossible
  case 6: return Case6();//impossible
  case 7: return Case7();//impossible
}

It doesn't matter that this is silly.

These are the rules, and when you choose to break them, you are choosing to write code that could stop working at any time, and only happens to work right now because the optimizer has not done a good enough job to break your invalid code, yet.

Choosing to write time-bombed code is silly. If you continue to use enums as flags after this, you're choosing to play chicken with your compiler. Good luck, and pray that it doesn't optimize as much as the spec says that it's allowed to.

This is very strange. Can someone point me to the part of the standard that says that a variable with an enum type can only hold values that are named constants? I have looked, and I haven't found it. Also, using powers of 2 as constants so you can do bit arithmetic is so common in C that I very much doubt C++ disallows it.

 

EDIT: I found this paragraph in section 7.2:

"For an enumeration whose underlying type is fixed, the values of the enumeration are the values of the underlying type. Otherwise, for an enumeration where e min is the smallest enumerator and e max is the largest, the values of the enumeration are the values in the range b min to b max , defined as follows: Let K be 1 for a two’s complement representation and 0 for a one’s complement or sign-magnitude representation. b max is the smallest value greater than or equal to max(|e min | − K, |e max |) and equal to 2 M − 1, where M is a non-negative integer. b min is zero if e min is non-negative and −(b max + K) otherwise. The size of the smallest bit-field large enough to hold all the values of the enumeration type is max(M, 1) if b min is zero and M + 1 otherwise. It is possible to define an enumeration that has values not defined by any of its enumerators. If the enumerator-list is empty, the values of the enumeration are as if the enumeration had a single enumerator with value 0."

 

In Hodgman's example, b min is 0 and b max is 7. So cases 0, 5, 6 and 7 are kosher.

 

 

I'm not trying to be a jerk here, but I don't see the standard supporting many of your claims on enumerations.  The relevant parts are in n4659 section 10.2 (also refer to section 8.2.9 (9) and (10)).

There is no contractual obligation to only store an enumerated value in an enumeration (10.2 (8) "For an enumeration whose underlying type is fixed, the values of the enumeration are the values of the underlying type").  You can store any value in an enum even if it is not explicitly enumerated provided the underlying type supports the value.  It even says in 10.2 (1) "An enumeration is a distinct type (6.9.2) with named constants.".  Its no different than a bunch of static const int's except that it obeys the type system.  The size of the underlying type is either explicitly specified, or determined according to 10.2 (5) (7) and (8).  As long as you stay within the range of the underlying type, your program will not be undefined.  The underlying bit pattern does not need a corresponding enumerated constant.

The compiler does not 'optimize' an enumeration any differently than any other type.  A switch on an enumeration, is the same as a switch on the underlying type.  It can't treat enum's as special constructs because static_cast is allowed (see 8.2.9 (9) and (10)).

You can static_cast a value back to an enumeration (8.2.9 (10)) provided that "the original value is within the range of the enumeration values"; and as per 10.2 (8) "the values of the enumeration are the values of the underlying type".

It is clear from the standard that enumerations are allowed to be treated as flags, that the underlying type must be (and is) well defined.  That storing bit patterns/values that do not have corresponding enumerations is well defined, and that operating on values using bit operations is well defined.

If you personally (or within your company) wish to use enum as a list of mutually exclusive options, then so be it.  But enum's are used for all sorts of integer constants (flags, options, counters, and half dozen other things), and are well defined in the spec to be capable of doing so.

Well at this point i have given up. Now i will use enums for single states only and use this for my flags - compiles in C and C++ (For C i require prefixing anyway...):


	typedef struct fpl_KeyboardModifierType {
		uint32_t value;
	} fpl_KeyboardModifierType;

	const uint32_t fpl_KeyboardModifierType_Alt = 1 << 0;
	const uint32_t fpl_KeyboardModifierType_Ctrl = 1 << 1;
	const uint32_t fpl_KeyboardModifierType_Shift = 1 << 2;
	const uint32_t fpl_KeyboardModifierType_Super = 1 << 3;

	inline fpl_KeyboardModifierType CreateKeyboardModifierType(uint32_t value) {
		fpl_KeyboardModifierType result;
		result.value = value;
		return(result);
	}

	static void doSomethingWithKeyboardModifiers(fpl_KeyboardModifierType modifiers) {
		if (modifiers.value & fpl_KeyboardModifierType_Ctrl) {
			// ...
  		}
	}

 

Sure i lost type safety that way, but its better than nothing...

Are you trying to find a way to write new enums that will compile as both C and C++ or are you modifying existing C code to compile with a C++ compiler?

The following method works for both and has type-checking in C++. Perhaps it will require less modifications to existing C code than adding operators to C++ code?:


#include <stdio.h>


typedef enum
{
	flag0,
	flag1,
	flag2,
	flag3 = 4,
	flag4 = 8
} test_flags;

void config( test_flags iflags[4] )
{
	unsigned uflags[] = { iflags[0], iflags[1], iflags[2], iflags[3] };

	printf( "0x%X\n", uflags[0] | uflags[1] | uflags[2] | uflags[3] );
}

int main( int argc, const char *args[] )
{
	test_flags my_flags[] = { flag1, flag2, flag3, flag4 };
	unsigned badflags[] = { 30, 40, 50, 32 };

	config( badflags ); // not accepted by C++
	config( my_flags );

	return 0;
}

 

This topic is closed to new replies.

Advertisement