StaticVector::StaticVector(const Vector&)

[Matthias Moulin]

Upon switching from my own fixed-size non-SIMD vectors to Blaze’s SIMD vectors in the context of strong types, I stumbled upon Blaze’s implicit StaticVector conversions being a bit too flexible imho.

Consider the below simplified example:

// Rgb is a derived class of blaze::StaticVector< float, 3u > having a.o. the following constructors:

Rgb(const Rgb& rgb) noexcept = default;

Rgb(Rgb&& rgb) noexcept = default;

explicit Rgb(const blaze::StaticVector< float, 3u >& rgb) noexcept
    : blaze::StaticVector< float, 3u >(rgb)
{}

explicit Rgb(blaze::StaticVector< float, 3u >&& rgb) noexcept
    : blaze::StaticVector< float, 3u >(std::move(rgb))
{}

// Rgba is a derived class of blaze::StaticVector< float, 4u > having a.o. the following cast operators:

explicit operator Rgb() const noexcept
{
    return { R(), G(), B() };
}

//

const Rgba rgba = { 1.0f, 2.0f, 3.0f, 4.0f };
const Rgb  rgb  = static_cast< Rgb >(rgba);

The last statement will not compile due to the presence of:

StaticVector<Type,N,TF>::StaticVector( const Vector<VT,TF>& v )

which in this concrete example tries to construct a blaze::StaticVector< float, 3u > from a blaze::StaticVector< float, 4u > in order to call one of the explicit Rgb constructors, which will fail at runtime. Instead, I would prefer the Rgba-to-Rgb cast operator to be the only viable choice in this case.

The latter could be achieved by using SFINAE/concepts/constraints to use the known size of StaticVector and StaticMatrixat compile time to eliminate potential conversions.

‌

I am not entirely sure why the cast operator is less appealing for the compiler, especially since my test suite worked fine with my fixed-size non-SIMD vectors (which have many conversion possibilities as well). So for equal size conversions, the problem remains.

Edit:

At least for a simple example the intended behavior is obtained: https://godbolt.org/z/9eY5Rv, but I’ll need to check an equal size conversion more thoroughly as some conversion is definitely interfering (or it is a compiler problem?).

[Matthias Moulin]

The reason why I use type cast operator:

Assume, one adds a different spectrum such as sRGB. In that case, one has to define a class, sRGB, that provides an explicit constructor taking a RGB and an explicit cast operator returning a RGB (while no changes have to be made to the RGB class itself).

[Matthias Moulin]

• Non-Blaze C++14: https://godbolt.org/z/E_w6dH
• Non-Blaze C++17: https://godbolt.org/z/dQB4MM
• Blaze C++14: https://godbolt.org/z/U-HwBE
• Blaze C++17: https://godbolt.org/z/enae8a

The C++14 <> C++17 difference (due to the guaranteed copy elision?) seems to be consistent across these samples.

And at least in Clang, I get my desired behavior in C++17 < … . (I know that I am playing with fire. )

Edit:

• Clang C++20: https://godbolt.org/z/A2YVKk
• MSVC C++20: https://godbolt.org/z/DgfsKN → This seems wrong for C++17 < …

‌

[Klaus Iglberger]

Hi Matthias!

I experimented with your code examples to find the reason for the behavior you describe. For me everything worked well, until I added line 4 in the following code example:

struct Rgb : StaticVector<float,3UL>
{
   using Base = StaticVector<float,3UL>;
   using Base::Base;  // Note this line of code!!!

   Rgb()
   {
      std::cerr << "Rgb()" << std::endl;
   }
   Rgb(const Rgb& rgb)
      : Base( rgb )
   {
      std::cerr << "Rgb(const Rgb&)" << std::endl;
   }
   Rgb(Rgb&&)
   {
      std::cerr << "Rgb(Rgb&&)" << std::endl;
   }
   explicit Rgb(const Vector&)
   {
      std::cerr << "Rgb(const Vector&)" << std::endl;
   }
   explicit Rgb(Vector&&)
   {
      std::cerr << "Rgb(Vector&&)" << std::endl;
   }
};

struct Rgba : StaticVector<float,4UL>
{
   Rgba()
   {
       std::cerr << "Rgba()" << std::endl;
   }
   explicit operator Rgb() const
   {
       std::cerr << "operator Rgb()" << std::endl;
       return Rgb{};
   }
};

int main()
{
   const Rgba rgba{ 1.0f, 2.0f, 3.0f, 4.0f };
   const Rgb rgb ( static_cast<Rgb>( rgba ) );  // Works as expected without line 4!
}

This line makes all constructors of the base class visible in the derived class. In this case the constructor taking a Vector is a better match and will be preferred. If this constructor is hidden, it is not considered and the conversion operator will be used.

Without additional information I assume that you have a similar line to make use of the constructors of StaticVector. If you remove this line, you should have the desired behavior.

However, you correctly state that in case I assign a StaticVector to another StaticVector of different size the error should be detected at compile time instead of at runtime. This is a very reasonable suggestion that we will introduce for both StaticVector and StaticMatrix. If we indeed remove this constructor from the overload set in the error case then your problem would also be solved. We will see what we can do.

Best regards,

Klaus!

[Matthias Moulin]

Hey Klaus,

I am not entirely sure whether the using directive makes the sample consistent across all compilers (and C++ standards), as this would have no impact for the last two samples (https://godbolt.org/z/A2YVKk <> https://godbolt.org/z/DgfsKN). The return value is different between MSVC and clang/gcc as the former does not use the cast operator, but converts the Xyz to a base class reference to use one of the explicit Rgb constructors instead. So Xyz::operator Rgb -> Rgb::Rgb(Rgb&&) and Rgb::Rgb(const Vector&)/Rgb::Rgb(Vector&&) are both 2 (explicit) sequences leading to a Rgb starting from a Xyz. But why would the latter ever be preferred in the presence of the cast operator?

Also note that including all constructors (including all the implicit ones) of the base class defeats the purpose of the strong type alias/typedef.

After reconsidering my original idea, I decided to not use strong types and to use free functions for the conversions instead, as this would be problematic for evaluating expressions with implicit conversions:

Rgb a;

Rgb b;

Rgb c = a + b; // Requires implicit constructor

‌

So wrt the issue a simple static_assert for the size check will suffice (as opposed to removing it from the overload set).

‌