c++ useless-cast from size_t to uint32_t for different targets

Viewed 366

I have some code that builds for different targets. It also has some legacy functions that take uint32_t instead of size_t - which is annoying when I want to cast size_t types to it - with the levels of warnings that we have set (lots of gcc warnings).

So here is a contrived example:

val32 = static_cast<uint32_t>(strings.size());
val64 = static_cast<uint64_t>(strings.size());  // ERROR

Depending on which arch this runs on, one of the two lines above will complain with useless cast warning (which we treat as errors). Now I know there are some ways around this like, change the code to take size_t... but, that is not my question. My question is, when this situation arises, how can I best tackle this.

I have come up with a solution - but it requires reference passing of variables:

template<typename TO, typename FROM>
TO static_cast_if_different(const FROM &value)
{
    if constexpr (std::is_same_v<TO, FROM>)
        return value;
    else
        return static_cast<TO>(value);
}

Now this works, but I feel there is a better way (perhaps built in to the standard - or an improvement of what I have done here)?

See the full example code here: https://godbolt.org/z/87hao1aTb

full list of warning flags for gcc: -Wall -Wextra -Wpedantic -Wconversion -Wsign-conversion -Wunreachable-code -Wlogical-op -Wshadow -Wmissing-include-dirs -Wparentheses -Wmisleading-indentation -Werror -Wno-psabi -Wno-error=deprecated-declarations -Wnon-virtual-dtor -Wuseless-cast -Wduplicated-cond -Wnull-dereference

Note: I don't want to remove the useless-cast warning flag as that is useful else where

2 Answers

It is a good solution, and it can still be improved somewhat.

  1. The cast and the std::is_same_v are not really necessary. A simple assignment will do exactly the same thing (when working with unsigned integral types, but we want to check for that). The function could look like this:

     template<typename TO, typename FROM>
     TO legacy_size_cast(FROM value)
     {
        static_assert(std::is_unsigned_v<FROM> && std::is_unsigned_v<TO>,
                      "Only unsigned types can be cast here!");
        TO result = value;
        return result;
     }
    

    You will still have a visual indication of a type conversion at the call site, which I suppose is what you're after. But if you do need an actual cast for some reason, you can still add it:

     TO result = static_cast<TO>(value);
    
  2. You can protect against accidental overflow by adding this:

     assert(result == value);
    

    This might not look like something terribly useful (who is going to allocate a single 4GB object?) but actually it can catch nasty bugs when you pass a negative offset, e.g. (uint64_t)(-1). This is actually a huge positive value, but it behaves like a -1 in arithmetic... until you convert it to uint32_t and then back to uint64_t, then it suddenly doesn't, and the assert will catch that.

  3. It does not matter if you pass by reference or by value. In a release build, the function is going to be inlined and optimized away completely anyway.

I would recommend using macros. E.g.

#ifdef SOME_ARCH
code
#else
other code
#endif

Then when you compile for some_arch, add -D SOME_ARCH to the compile flags.

Related