Prevent instantiation of CRTP base class objects which are not a part of an object of a derived class

[Mikhail Katliar]

Currently the following (erroneous) code compiles:

#include <blaze/Math.h>
#include <iostream>

template <typename MT, bool SO>
void foo(blaze::Matrix<MT, SO> m)
{
    std::cout << ~m;   // garbage!
}

int main(int, char **)
{
    using namespace blaze;
    StaticMatrix<double, 4, 5, rowMajor> A(0.);

    foo(A);

    Matrix<StaticMatrix<double, 4, 5, rowMajor>, rowMajor> B;
    std::cout << B; // garbage!

    return 0;
}

Objects of CRTP base classes such as Vector, Matrix, DenseVector etc. should never be instantiated if they are not a part of an object of a derived class such as DynamicVector, DynamicMatrix etc. This can be ensured at compile-time by making default and copy constructors of the above classes protected.

[Mikhail Katliar]

Proposed fix here: https://bitbucket.org/blaze-lib/blaze/pull-requests/40

[Klaus Iglberger]

Hi Mikhail!

Thanks a lot for pointing out this shortcoming. I completely agree, all CRTP base classes should not be instantiable.

Also thanks for the pull request. I agree that declaring both the default and copy constructor as protected prevents the instantiation. However, I prefer to declare the destructor as protected as this is only one function. Also, since DenseVector and SparseVector are derivatives of Vector and since DenseMatrix and SparseMatrix are derivatives of Matrix, it is sufficient to deal with Vector and Matrix only.

Thanks a lot. Thanks to you Blaze is indeed becoming a much better library!

Best regards,

Klaus!

[Klaus Iglberger]

After some testing I found that just declaring the destructor as protected is not the perfect solution anymore. Some compilers (namely Clang) warn about the fact that the default generation of the copy constructor is deprecated. Thus I have fallen back to Core guidelines C.21 and declared all special member functions as default.

[Klaus Iglberger]

Commit cf202d0 prevents the instantiation of all CRTP base classes in the Blaze library. This protection is immediately available via cloning the Blaze repository and will be officially released in Blaze 3.7.

[Mikhail Katliar]

Hello Klaus,

Thanks for taking care of this issue. However, after your fix it the following erroneous code still compiles:

#include <blaze/Math.h>
#include <iostream>

template <typename MT, bool SO>
void denseFoo(blaze::DenseMatrix<MT, SO> m)
{
    std::cout << ~m;   // garbage!
}

int main(int, char **)
{
    using namespace blaze;

    StaticMatrix<double, 4, 5, rowMajor> A1(0.);
    denseFoo(A1);

    DenseMatrix<StaticMatrix<double, 4, 5, rowMajor>, rowMajor> A2;
    std::cout << A2; // garbage!

    return 0;
}

As far as I understand, declaring special member functions as protected in a base class does not prevent the compiler from auto-generating public special member functions in a derived class. So it looks like they need to be re-declared as protected in the intermediate classes as well.

‌

[Klaus Iglberger]

You are absolutely correct and I apologise for this oversight. Commit 1787870 takes care of all intermediate CRTP classes (DenseVector, SparseVector, DenseMatrix, SparseMatrix and Proxy). Thanks,

Best regards,

Klaus!

[Klaus Iglberger]

Commit 1787870 prevents the instantiation of all intermediate CRTP base classes in the Blaze library. This protection is immediately available via cloning the Blaze repository and will be officially released in Blaze 3.7.