2
\$\begingroup\$

I am implementing a generic absolute value function that handles signed and unsigned integer values correctly over the input type's domain. The std::abs(...) function leaves the case of std::abs(INT_MIN) as undefined behavior cppreference. I hope to resolve the undefined behavior by representing the returned absolute value in the input type's corresponding unsigned type. For example,

int input = INT_MIN; // -2147483648
unsigned output = integral::abs(input);

will correctly return 2147483648, but as an unsigned type.

To accomplish this I implemented the following,

    namespace integral
    {
        template<typename integral_value_t>
        auto
        abs(integral_value_t const val)
        {
            static_assert(std::is_integral<integral_value_t>::value);
            if(val < 0)
            {
                return static_cast<
                       typename std::make_unsigned<integral_value_t>::type>(-val);
            }
            return static_cast<
                       typename std::make_unsigned<integral_value_t>::type>(val);
        }
    }

but it created some concerns.

Concerns:

  1. Returning an unsigned type creates a type management challenge for the caller.
  2. Maybe there is a better name for the function or namespace?
  3. Is the typename keyword required in static_cast<typename(gcc 12.1 complained)?
  4. Can the make_unsigned<...> call be made once? Making it twice is overly verbose.

The code is also available on godbolt.

\$\endgroup\$
4
  • \$\begingroup\$ One thing you could do, not long enough to be an answer, is integral_value_t new_value = val < 0 ? -val : val;, and then return the make_unsigned<...> with that new value. \$\endgroup\$ Commented Aug 20, 2022 at 23:45
  • \$\begingroup\$ Thanks for the comment, it got me thinking about using a using statement to store the type. For example using return_t = std::make_unsigned<integral_value_t>(val)::type. See godbolt for concrete example. \$\endgroup\$ Commented Aug 20, 2022 at 23:53
  • \$\begingroup\$ @Linny That would not work. Consider the type of the result of the expression -val if val is an int and its value is INT_MIN. Also see this post. \$\endgroup\$ Commented Aug 21, 2022 at 9:02
  • 1
    \$\begingroup\$ BTW, there's no template-meta-programming here - it's just straightforward template code. I've edited tags appropriately. \$\endgroup\$ Commented Aug 22, 2022 at 20:23

1 Answer 1

Reset to default
1
\$\begingroup\$

To answer your specific concern (3): yes, the typename keyword is required, because at parse time the type std::make_unsigned<integral_value_t> isn't known, so its type member could be an object as far as the compiler is concerned. It's only once integral_value_t is known that it becomes a complete type.

For concern (4), I'd recommend a using statement to avoid repeating the long type name.

We should include the <type_traits> header so that our function is usable immediately.

Instead of the static_assert, I'd use a constraint to make the non-integral overloads disappear. Perhaps:

auto abs(std::integral auto&& val)

It might even be worth constraining to accept only signed types, though that could inhibit its use in generic code.

It looks like we're assuming 2's-complement representation, given that -INT_MAX is undefined in standard C++. The if/else seems to be duplicating std::abs(), so why not call that? It's no more undefined than -val is.

To eliminate undefined behaviour, we can't simply use -val in all cases. I'd do it by adding to val until it is large enough to be negated without overflow:

#include <concepts>
#include <limits>
#include <type_traits>

namespace integral
{
    template<std::unsigned_integral T>
    auto abs(T&& val)
    {
        return val;
    }

    template<std::signed_integral T>
    auto abs(T&& val)
    {
        using U = typename std::make_unsigned_t<T>;

        if (val >= 0) {
            return static_cast<U>(val);
        }

        static constexpr auto maxval = std::numeric_limits<T>::max();
        U retval = 0;
        while (val < -maxval) {
            val += maxval;
            retval += maxval;
        }
        return retval - val;
    }
}

I think it's guaranteed that the while will only ever perform one iteration, but I can't find where the standard says that negative range of signed types can only be slightly greater than their positive range.

\$\endgroup\$
5
  • \$\begingroup\$ Ouch, that while-loop is super confusing to read. And is it even correct? If you need to increment val twice, wouldn't that be very likely to cause retval to overflow? \$\endgroup\$ Commented Aug 21, 2022 at 12:06
  • \$\begingroup\$ It shouldn't do (as retval is of the unsigned type). I'm not sure it's even permitted for INT_MIN to be less than -2 * INT_MAX, or the equivalent for any other signed type. Do you have a suggestion for making it clearer (perhaps change the condition to val + maxval < 0? Or better variable names?) or for an alternative, but safe, implementation? \$\endgroup\$ Commented Aug 21, 2022 at 14:49
  • 1
    \$\begingroup\$ "I can't find where the standard says that negative range of signed types can only be slightly greater than their positive range." Is there equivalent guaranty that -std::numeric_limits<T>::max() is in range? :-) \$\endgroup\$ Commented Aug 22, 2022 at 7:02
  • \$\begingroup\$ That's a good question too, @Jarod. I guess some detailed standards-diving is required for a completely robust implementation; unfortunately I don't have time to do that right now. :-( \$\endgroup\$ Commented Aug 22, 2022 at 7:08
  • 2
    \$\begingroup\$ eel.is/c++draft/basic.fundamental#1 for C++20 (2's complement mandatory). \$\endgroup\$ Commented Aug 22, 2022 at 7:17

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.