views:

3495

answers:

4

I found this in the code I'm working on at the moment and thought it was the cause of some problems I'm having.

In a header somewhere:

enum SpecificIndexes{
    //snip
    INVALID_INDEX = -1
};

Then later - initialization:

nextIndex = INVALID_INDEX;

and use

if(nextIndex != INVALID_INDEX)
{
    //do stuff
}

Debugging the code, the values in nextIndex didn't quite make sence (they were very large), and I found that it was declared:

unsigned int nextIndex;

So, the initial setting to INVALID_INDEX was underflowing the unsigned int and setting it to a huge number. I assumed that was what was causing the problem, but looking more closely, the test

if(nextIndex != INVALID_INDEX)

Was behaving correctly, i.e, it never executed the body of the if when nextIndex was the "large +ve value".

Is this correct? How is this happening? Is the enum value being implicately cast to an unsigned int of the same type as the variable, and hence being wrapped in the same way?

Cheers,

xan

+1  A: 

In fact, -1 is implicitly cast to its equivalente unsigned value when it is assigned to nextValue. The equivalente unsigned is the value with the same bitwise representation (which is 111111111111..., this is, the maximum unsigned value).

Later on, in the comparison statement, another implicit cast happens.

So this works right now, but may cause problem in the future. It is never a good idea to mix signed and unsigned values.

Gorpik
And what about the test with the enum value, I assume that's an implicit cast too?
xan
Just for the record, I agreed with xan's point and edited my entry accordingly. But I don't know why my comment has disappeared.
Gorpik
"The equivalent unsigned is the value with the same bitwise representation" - this is true if the implementation uses two's-complement representation for signed integral types. However it's not the definition, and is not true for one's-complement or sign-value representations. Which are rare anyway.
Steve Jessop
+6  A: 

Yes to everything. It is valid code, it is also commonly used library-side C++ code, more so in modern C++ (it is strange when you see it the first time but its a very common pattern in reality).

Then enums are signed ints, but they get implicitly cast into unsigned ints, now this depending on your compiler might give a warning, but its still very commonly used, however you should explicitly cast to make it clear to maintainers.

Robert Gould
I heartily disagree. First, your code says that nextIndex is unsigned, when in fact it is not, misleading the reader. Second, if you later decide to add new positive values to SpecificIndexes and use operators > or < for your checks, nothing will work.
Gorpik
enums are signed, but you are right about the > < operators screwing up, such as if you sort them in a container. However a good library would handle this case safely, or not let you add an invalidIndex to the container in the first place. Anyways you'll see stuff like this even in Boost...
Robert Gould
Yes - this is a problem I'm finding. Functions that use nextIndex Assert that it's > 0, so changing it to use a signed int is not a trivial option, and other code I think relies on the way it has been implemented. It's rather confusing.
xan
smells like time to refactor to me :) Anyways a quick solution is to Assert(index>0 That is correct at least and makes the assertion better documented too.
Robert Gould
A: 

Yes, I believe enums are signed. Change

unsigned int nextIndex;

to

int nextIndex;

and your program should work.

It works as it is (it's part of an existing system, not something I'm writing from scratch) - my question was *why* it worked as it did as it looked very wrong!
xan
A: 

enums may be represented by signed or unsigned integral types according to whether they contain any negative values and what the compiler feels like. The example here contains a negative value and hence must be represented by a signed integral type.

Equality comparison between signed and unsigned types is safe and usually does what the author intended - the signed value will be converted to unsigned first, and the result of doing that is defined by the C++ standard and is intuitive (at least, it is once you know the destination type. Except maybe if integers aren't two's complement. So maybe not that intuitive, but it doesn't normally cause problems).

Order comparison is more likely to result in errors. For example:

SpecificIndexes value = INVALID_VALUE;
return (value >= 0);

returns false, but:

unsigned int value = INVALID_VALUE;
return (value >= 0);

returns true. Sometimes the author will not appreciate the difference, especially if the type of "value" isn't quite so obvious at the point of use. The compiler may well warn about the second example, though, because (value >= 0) is a tautology.

Steve Jessop