0

I have seen some other answers that propose to use unions for byte swapping (which is UB or can't be done at compile time).

I've wrote mine, and it kind of worked until I have met some case which have shown that my implementation is invalid. I can't spot a mistake, can you help me?

namespace impl
    {
                // ENDIAN is defined via CMake TestBigEndian
        constexpr bool native_is_big_endian()
        {
#ifdef ENDIAN
            return true;
#else
            return false;
#endif
        }
    }

    /*!
     * \brief std compliant type for endianness
     * \details
     * If all scalar types are little-endian, endian::native equals endian::little
     * If all scalar types are big-endian, endian::native equals endian::big
     */
    enum class endian
    {
        little,
        big,
        native = impl::native_is_big_endian() ? big : little
    };

    template<typename T>
    class swap_endian
    {
        constexpr static size_t sz_minus_one = sizeof(T) - 1;
        template<size_t> struct tag_s
        {
        };

        constexpr static T bitwise_or(tag_s<0>, T original, T res)
        {
            return res | (original >> sz_minus_one * 8);
        }

        template<size_t i>
        constexpr static T bitwise_or(tag_s<i>, T original, T res)
        {
            return bitwise_or(tag_s<i - 1>(), original, original << i * 8 >> sz_minus_one * 8 << i * 8);
        }

    public:
        constexpr static T swap(T u)
        {
            return bitwise_or(tag_s<sz_minus_one>(), u, 0);
        }
    };

    template<typename T>
    constexpr T swap_endian_v(T u)
    {
        return swap_endian<T>::swap(u);
    }

    template<endian From, typename T>
    constexpr T to_native_endian(T u)
    {
        return From == endian::native ? u : swap_endian_v(u);
    }

int main()
{
    static_assert(uint8_t(0xFA) == swap_endian_v(uint8_t(0xFA)), "Invalid result for endian swapping");
    static_assert(uint16_t(0x00AA) == swap_endian_v(uint16_t(0xAA00)), "Invalid result for endian swapping");
    static_assert(uint16_t(0xF0AA) == swap_endian_v(uint16_t(0xAAF0)), "Invalid result for endian swapping");
    static_assert(uint32_t(0x00'00'CC'00) == swap_endian_v(uint32_t(0x00'CC'00'00)),
                  "Invalid result for endian swapping");

// this fails
//    static_assert(uint32_t(0x6A'25'65'75) == swap_endian_v(uint32_t(0x75'65'25'6A)),
//                  "Invalid result for endian swapping");
    return 0;
}

Please, don't suggest to use BOOST. At this point I am very interested in finding out what kind of mistake I've made in the algorithm.

Sergey Kolesnik
  • 3,009
  • 1
  • 8
  • 28
  • 4
    Wouldn't `uint16_t(1) >> 8u` evaluate to 0 independently of the endianness of the current architecture? I don't think there is any portable compile-time test for endianness until `std::endian` from C++20. – Daniel Langr Nov 26 '20 at 16:36
  • @DanielLangr thank you for pointing this out! – Sergey Kolesnik Nov 26 '20 at 16:44
  • 1
    The output of `swap_endian_v(uint32_t(0x75'65'25'6A))` is `6575` which means the higher order bytes are getting discarded – Harry Nov 26 '20 at 16:47
  • @Harry I am aware of that. I can't see the error in the code. – Sergey Kolesnik Nov 26 '20 at 16:47
  • I’m voting to close this question because it's asking for a review of existing code rather than a solution to a specific problem. – ecatmur Nov 26 '20 at 17:05
  • @ecatmur this is ridiculous. I am looking for a solution for a specific problem, which is convert endianness at compile time. I have said that I haven't found constexpr solutions. – Sergey Kolesnik Nov 26 '20 at 17:09
  • why do you think `native_is_big_endian()` is correct? – Marek R Nov 26 '20 at 17:10
  • @MarekR I don't, since I've read Daniel's comment. I realized later that this approach makes no sense... Now I am using CMake to define a preprocessor variable for big endian. – Sergey Kolesnik Nov 26 '20 at 17:12
  • @SergeyKolesnik sorry, it wasn't clear from the question what the failing case was. See https://stackoverflow.com/help/minimal-reproducible-example – ecatmur Nov 26 '20 at 18:39

1 Answers1

2

You are ignoring the third argument passed into the recurrent overload of bitwise_or through the res parameter. It seem to work if

return bitwise_or(tag_s<i - 1>(), original,
  original << i * 8 >> sz_minus_one * 8 << i * 8);

is changed to:

return bitwise_or(tag_s<i - 1>(), original,
  res | original << i * 8 >> sz_minus_one * 8 << i * 8);

Live demo: https://godbolt.org/z/xW81z4

Daniel Langr
  • 22,196
  • 3
  • 50
  • 93