compiler bug or bad code?

[kraftcheck]

I have the following code:

#include <iostream>

enum MyEnum { A = 0, B, LAST };

inline MyEnum & operator++(MyEnum &e)
{
return reinterpret_cast<MyEnum&>(++reinterpret_cast<int&>(e));
}

int main()
{
for (MyEnum i = A; i < LAST; ++i)
std::cout << i << std::endl;
return 0;
}

When compiling this with G++ 4.1, the output for a non-optimized build
is:
0
1

If I compile with -O2, I get
0
0
1

Is there something wrong with the way the ++ operator is defined?

 

[Thomas Tutone]

I have the following code:

#include <iostream>

enum MyEnum { A = 0, B, LAST };

inline MyEnum & operator++(MyEnum &e)
{
return reinterpret_cast<MyEnum&>(++reinterpret_cast<int&>(e));
}

int main()
{
for (MyEnum i = A; i < LAST; ++i)
std::cout << i << std::endl;
return 0;
}

When compiling this with G++ 4.1, the output for a non-optimized build
is:
0
1

If I compile with -O2, I get
0
0
1

Is there something wrong with the way the ++ operator is defined?

To answer the question in your subject heading, "Bad code."

Change operator++ to the following:

inline MyEnum& operator++(MyEnum& e)
{
return e = static_cast<MyEnum>(static_cast<int>(e) + 1);
}

Don't worry - the optimizer will make it plenty efficient.

Best regards,

Tom

 

[Frederick Gotham]

posted:

Is there something wrong with the way the ++ operator is defined?

Undefined behaviour arises out of:

reinterpret_cast<int&>(e)

Which is equivalent to:

*reinterpret_cast<int*>(&e)

The Standard provides no guarantee that the enum type shall be represented as
an "int" in memory.

If anything, I'd probably write it something like:

inline MyEnum &operator++(MyEnum &e)
{
int val = e;
return e = (MyEnum)++val;
}

 

[kraftcheck]

To answer the question in your subject heading, "Bad code."

Change operator++ to the following:

inline MyEnum& operator++(MyEnum& e)
{
return e = static_cast<MyEnum>(static_cast<int>(e) + 1);
}

Don't worry - the optimizer will make it plenty efficient.

Thanks. I had already tried the alternate implementation you mention
above, and noticed that it fixed the problem. However, I am still
curious as to why the implementation I originally had didn't work.
Would you mind elaborating on why my implementation was wrong?

Thanks much,

Jason

 

[Thomas Tutone]

(e-mail address removed) wrote:

[snip]

To answer the question in your subject heading, "Bad code."

Change operator++ to the following:

inline MyEnum& operator++(MyEnum& e)
{
return e = static_cast<MyEnum>(static_cast<int>(e) + 1);
}

Don't worry - the optimizer will make it plenty efficient.

Thanks. I had already tried the alternate implementation you mention
above, and noticed that it fixed the problem. However, I am still
curious as to why the implementation I originally had didn't work.
Would you mind elaborating on why my implementation was wrong?

Fred Gotham already answered your question - there is no guarantee that
an enum is being represented by an int - it may be represented by a
char or a short, for example. Therefore your reinterpret_cast - which
is almost always a bad idea unless you are doing something both very
low-level and implementation specific (and you're not doing so here) -
can treat something that may only be of size 1 as if it were, say, of
size 4 (depending on your implementation) - and whatever random data is
there. Or imagine it in reverse - the bit pattern for a size 4 int is
stuck into an enum of size 1.

In short - static_cast "does the right thing" in these circumstances.
reinterpret_cast does not - it says to the compiler, "Trust me - I know
what I'm doing" - whether you do or not.

Best regards,

Tom