expanded constexpr support in Ogre::Vector

[ribbon-otter]

This initial version adds support for constexpr indexing into vectors and comparing them with ==, !=, <, and > on C++20 versions and above. My goal is to add support for Ogre::Vector math in a constexpr context as much as is possible.

Because I wanted to try compiling code in a constexpr context with various versions of C++, I created the CppVersionTesting folder and along with a shell script to automatically check the validity the code on each version of C++ between 14 and 23 inclusive. The script currently only g++ (and likely clang++). How do you feel about the script and testing file? do you want it rewritten in Cmake (I tried at first but it seemed not easy) or perhaps excluded from the project all together?

Strictly speaking the code works without the OGRE_CPP20_CONSTEXPR macro to remove constexpr for C++ versions less than c++20. However, the following paragraph from cppreference makes me think that it makes the program ill-formed.

For a templated constexpr function, if no specialization of the function/class template would make the templated function constexpr-suitable when considered as a non-templated function, the program is ill-formed, no diagnostic required.

Moreover, it makes error messages shorter on non-supported C++ versions with a not constexpr method error when you try to use it rather than a deeper error explaining why the constexpr function is invalid.

OGRE_CPP23_CONSTEXPR is currently unused, and will be removed in the final version if I don't find need for the extra powers that .

This code places the data array in an anonymous union with x,y,z,w data members in the legacy Vector bases. Accessing a union from it's inactive path is officially undefined; however, to my knowledge, it is defined as doing the obvious thing in all the major compilers. I am unsure if the strategy of doing pointer math inside a homogeneous struct currently used is defined according to the spec either.

Does this look like an acceptable direction for things to go from your perspective as a maintainer, or should I look for an alternative strategy for constexpr support?

fixes #3605

[ribbon-otter]

oh, if we choose to keep the test file and the testing script, what do you think would be the appropriate copyright header to include at the top?

[paroj]

wouldnt making ptr() constexpr work too?

[ribbon-otter]

perhaps I didn't do the correct combination when I tried, but that was my first strategy, and I didn't have success with it. Presumably because constexpr is a safe context where you can't do UB (to my knowledge). This is why I had to change

 constexpr VectorBase(Real _x, Real _y, Real _z) : x(_x), y(_y), z(_z) {}

to

constexpr VectorBase(Real _x, Real _y, Real _z) : data{_x, _y, _z} {}

Since in a constexpr context, the for-loops would refuse access members of a union from the non-initialized path otherwise.

[paroj]

anonymous structs are non-standard c++

[ribbon-otter]

ah, good catch; that explains why I learned about them from reading a difference open-source game engine's source code rather than from reading Cpp reference.

[paroj]

This code places the data array in an anonymous union with x,y,z,w data members in the legacy Vector bases. Accessing a union from it's inactive path is officially undefined; however, to my knowledge, it is defined as doing the obvious thing in all the major compilers. I am unsure if the strategy of doing pointer math inside a homogeneous struct currently used is defined according to the spec either.

you are correct, that doing pointer math on Vector is UB too. However as it is what we are currently doing, I would rather continue with that instead of doing other UB stuff.

How do you feel about the script and testing file? do you want it rewritten in Cmake (I tried at first but it seemed not easy) or perhaps excluded from the project all together?

if code is not executed, you can assume it is broken. I would rather have a gtest guarded behind "#if CPP20" and switch our custombuild to CPP20 for that, so it is actually executed.

The rest looks good to me.

[ribbon-otter]

I suppose using gtest works. I didn't use that because the error would show up at compile time (since doing the calculation at compile time is the feature; and static_assert is the equivalent to a unit test doing the "execution"). However, the static_asserts and constexprs just need to be some code that gets compiled, and so they could go inside a gtest test even if the test itself is a bit trivial. (contrary to it's name the '--syntax-only' option on gcc actually runs and tests the static_asserts).

[ribbon-otter]

I think I will try make a second attempt at this idea using std::bit_cast instead of the anonymous union with an anonymous struct, since std::bit_cast is part of the standard, though it is a c++20 feature which is less good.

[paroj]

another angle would be to fix it in operator[] like:

// In VectorBase<3, Real>
Real& operator[](size_t i) {
    assert(i < 3);
    switch(i) {
        default:
        case 0: return x;
        case 1: return y;
        case 2: return z;
    }
}

[ribbon-otter]

That would work but my concern is performance degradation, since it compiles to code with branches or at least individually checking each case.

I am leaning towards thinking that constexpr on Vector isn't possible without writing a separate implementation of Vector for c++20 compilers, or with the non-standard anonymous struct and union solution I proposed above. (though anonymous structs appear to be supported on gcc, clang, and msvc, so it is possible that using them won't cause problems in practice).