SafeCast with enumeration and expected truncation fails

[jacobl-at-ms]

Up front, Trying to use SafeInt to cast to an enum seems like something that should be rejected at compile time to me (possibly through the use of static_assert but I can also see an argument to look at the underlying storage size. What I didn't expect was to see the enum get through to std::numeric_limits<> which just returned 0.

Example I have is:

#include "SafeInt.hpp"
#include <cstdint>
#include <iostream>

enum MyEnum : int {
};

int main(uint64_t argc)
{
    MyEnum x;
    std::cout << "SafeCast(u, tT): " << SafeCast(argc, x) << "\n";
    return 0;
}

compiled with:

cl /std:c++20 /nologo /Wall /EHsc /Zi example.cpp

prints:

prompt>example.exe
SafeCast(u, tT): 0

The "problem" seems to be that std::numeric_limits<> on an enum type will always return 0/false even if it could safely cast the value.

When I step into the debugger I see this stack:

>	example.exe!SafeCastHelper<enum MyEnum,unsigned __int64,2>::Cast(unsigned __int64 u, MyEnum & t) Line 1115	C++
 	example.exe!SafeCast<unsigned __int64,enum MyEnum>(const unsigned __int64 From, MyEnum & To) Line 5385	C++
 	example.exe!main(unsigned __int64 argc) Line 11	C++

and we end up returning in this block:

        if( u > (U)std::numeric_limits<T>::max() )
            return false;

because max() returns 0.

[dcleblanc]

This is going to be a bit tricky. In classic C, we could assume that you can always cast an int to an enum and vice versa. Now you could have an enum derived from any integral type. Having numeric_limits not understand that is inconvenient.

We do have enum detection built into the class, like so:

isInt = std::is_integral::value || std::is_enum::value,

Though I suspect that:

enum the_enum : int { foo, bar, baz };
SafeInt<the_enum> s;

Will go very badly and should probably have a static assert to prevent that.

But this will work:

template < typename U >
SAFEINT_CONSTEXPR14 SafeInt( const U& i ) SAFEINT_CPP_THROW : m_int(0)

And that invokes SafeCast, which knows about enums. Though what it does is cast to an int, which might not be always correct. See SafeCastHelper, CastFromEnum.

Which we're apparently not using. I suspect that since your enum is a derived class from an int, it is getting detected as an actual int.

I'll do a bit more digging, if you could also help out by probing how these work with the newer numeric templates, I'd appreciate it.

[dcleblanc]

Also, TIL, there is this - std::underlying_type

I think this can allow us to solve this in a nice way. There's also a to_underlying, but that's only in C++23.

[dcleblanc]

@jacobl-at-ms

you're trying to cast TO an enum, not FROM an enum. That's not supported.

Please go put a static_assert in SafeCast to ensure that To is not an enum, fix your build breaks, let me know when done, then I'll add the static_assert to the code.

I might consider a feature request to treat an incoming enum as its underlying type, there are ways to get that, and if prior to C++11, I can force it to an int.

[dcleblanc]

@jacobl-at-ms - this is seeming like a design question. Like you, I didn't expect numeric_limits to fail, either. We can take two approaches to this:

1. Make a static assert that the To argument is not an enum. User could cast manually to underlying enum type and check range.
2. We can also go find out the type of the enum. There is a C++11 way to get the underlying type. Prior to C++11 is still supported, but in that case we could just assume int. We could then cast to the underlying type, and obviously we'd have no idea whether that's a legitimate value for the enum.

The first is a little easier and safer. What is your preference?

[jacobl-at-ms]

Sorry for the delayed response. I'm actually working on adding the static_assert that things aren't an enum to every location where numeric_limits<> is used in our local copy already so I think that would be my preference. I'm currently hung up on some internal org validation behaving badly though. I'm hoping to be able to share a diff with you soon of the few changes we've needed to make so that you can see what our needs have been more concretely.

Trying to do the "right thing" with enums seems just a bit odd and error prone to me. Not all enum groups are really meant to be used arithmetically; it doesn't necessarily make sense that enumValueX + enumValueY is always logically valid and so it doesn't make sense to me that SafeInt would operate on those types of enumerations. If they're meant to be flags then the enum might have bitwise operations but not arithmetic operations defined, for example.

[dcleblanc]

So this:

enum foo {a };

SafeInt

Should fail to compile, so it shouldn't be as horrible as you might think.

What should work, though is:

SafeInt x(foo::a);

Because conversion from enum to int is intentionally supported.

If somehow SafeInt does compile, we need to fix that.

Then this:

SafeInt x;
x += foo;

Probably works, and is weird, but I can't see that happening much.

What I did miss is casting TO an enum, but I think that's only the SafeCast path.

[dcleblanc]

See PR #65

[jacobl-at-ms]

The static_asserts seem completely fine to me. I know we compile and validate now with static_asserts in place and had only 2 places where we hit this issue. It's pretty minor and edge case-y but would appreciate the upstream having the validation so that we can replace our validation with whatever the upstream publishes.

master...jacobl-at-ms:SafeInt:master should give you an idea of what we've needed to change to get up-to-date. It is mostly the static_casts for this issue but also a changes related #51, #61, and some macro shuffling so that we can have the multiple validation types seen at once.

[jacobl-at-ms]

thanks github for not showing me that PR response before I replied. As soon as that hits master I'll look at removing our changes for this issue and moving to only what is in upstream.