Has CRTP no compile time check?

Question

I was trying to implement static polymorphism using the Curiously Recurring Template Pattern, when I noticed that static_cast<>, which usually checks at compile time if a type is actually convertible to another, missed a typo in the base class declaration, allowing the code to downcast the base class to one of its siblings:

#include <iostream>

using namespace std;

template< typename T >
struct CRTP
{
    void do_it( )
    {
        static_cast< T& >( *this ).execute( );
    }
};

struct A : CRTP< A >
{
    void execute( )
    {
        cout << "A" << endl;
    }
};

struct B : CRTP< B >
{
    void execute( )
    {
        cout << "B" << endl;

    }
};

struct C : CRTP< A > // it should be CRTP< C >, but typo mistake
{
    void execute( )
    {
        cout << "C" << endl;
    }
};

int main( )
{
    A a;
    a.do_it( );
    B b;
    b.do_it( );
    C c;
    c.do_it( );
    return 0;
}

Output of the program is:

A
B
A

Why does the cast work with no errors? How can I have a compile time check that could help from this type of errors?

Lynnlynna

You can static_cast to derived classes. You can’t check at compile time what the runtime type will be. – Twicetold

@MooingDuck, It's good, but it doesn't catch passing the wrong derived type like in the question. But to catch that, I think CRTP would have to be supported in the language anyway (e.g., mixins). – Hargreaves

Oh, whoops. You're right. For some reason I was thinking one of those would be the C, but they're both A. Maybe if a variant went in the derived class? static_assert(std::is_base_of<CRTP<decltype(this)>,decltype(this)>::value)? – Woodcraft

@MooingDuck, Yeah, I think that's the best you're going to get today. With metaclasses, I believe you could at least have some sort of crtp<CRTP> C { ... }; and not need to inherit manually at all. That said, I think metaclasses lends itself more to either combining the desired mixins into a new metaclass or some general with_mixins<...> Foo { ... }; – Hargreaves

Both A and C are derived from CRTP<A>, so a static_cast is allowed from CRTP<A> to either of them. A cast to an unrelated (or ambiguous) type, e.g. B would require a diagnostic, though. – Epictetus

Answer 1

The usual way to solve this in CRTP is to make the base class have a private constructor, and declare the type in the template a friend:

template< typename T >
struct CRTP
{
    void do_it( )
    {
        static_cast< T& >( *this ).execute( );
    }
    friend T;
private:
    CRTP() {};
};

In your example, when you accidentally have C inherit from CRTP<A>, since C is not a friend of CRTP<A>, it can't call its constructor, and since C has to construct all its bases to construct itself, you can never construct a C. The only downside is that this doesn't prevent compilation per se; to get a compiler error you either have to try to actually construct a C, or write a user defined constructor for it. In practice, this is still good enough and this way you don't have to add protective code in every derived as the other solution suggests (which IMHO defeats the whole purpose).

Live example: http://coliru.stacked-crooked.com/a/38f50494a12dbb54.

NB: in my experience, the constructor for CRTP must be "user declared", which means you cannot use =default. Otherwise in a case like this, you can get aggregate initialization, which will not respect private. Again, this might be an issue if you are trying to keep the trivially_constructible trait (which is not a terribly important trait), but usually it shouldn't matter.

Guanajuato

Doesn't this allow one to do something silly like void MyBadClass::memberFunction() {CRTP<MyBadClass> x; x.do_it();}? – Disafforest

@JoshuaGreen Well, the constructor was public in the original solution, so if you wanted to construct the CRTP class standalone, nothing was stopping you at all. So, I wouldn't say it "allows" you to do something silly relative to the original situation. – Guanajuato

That's fair. I thought I had a solution to this, but I'm struggling to remember it at the moment. – Disafforest

@Joshua Green you could give it an abstract method. Then it cannot be instantiated alone, only as a base. But then you are adding a vtable, and boilerplate perhaps. Protect against Murphy, not Machiavelli. – Guanajuato

It does not compile (even in your link). – Goddamned

@Goddamned it's supposed to not compile, suggest rereading the question and answer. – Guanajuato

I know why it does not compile on my side. I have Abstract<T> : CRTP<T> and then Concrete : Abstract<Concrete>. But because Abstract<T> is not a friend of CRTP<T> it does not compile. Is there a way to solve this? – Goddamned

@Goddamned yes, use the pattern correctly: Abstract<T> : CRTP<Abstract<T>>. That said, there is no reason to have an "abstract" class in C++ static polymorphism. – Guanajuato

Answer 2

Q1 Why does the cast work with no errors?

When none of the sensible things apply ...

From https://timsong-cpp.github.io/cppwp/n3337/expr.static.cast#2:

Otherwise, the result of the cast is undefined.

---

Q2 How can I have a compile time check that could help from this type of errors?

I was not able to find a method that can be used in CRTP. The best I could think of is to add static_assert in the derived classes.

For example, if you change C to:

struct C : CRTP< A > // it should be CRTP< C >, but typo mistake
{
   static_assert(std::is_base_of<CRTP<C>, C>::value, "");
   void execute( )
   {
      cout << "C" << endl;
   }
};

You will see the error at compile time.

You can simplify that to

struct C : CRTP< A > // it should be CRTP< C >, but typo mistake
{
   using ThisType = C;
   static_assert(std::is_base_of<CRTP<ThisType>, ThisType>::value, "");
   void execute( )
   {
      cout << "C" << endl;
   }
};

Similar code needs to be added in each derived type. It's not elegant but it will work.

PS I wouldn't recommend using the suggested solution. I think it's too much overhead to account for the occasional human error.

Voyageur

"How can I have a compile time check that could help from this type of errors?" is really the key part of the question – Woodcraft

@MooingDuck, I copied the solution from your comment. Hope you don't mind. – Voyageur

I wouldn't mind, but chris observed that it's wrong – Woodcraft

Where should I write the static_assert? I added it as first line of the do_it() method but it still compiles despite of the typo. – Lynnlynna

@nyarlathotep108: Try static_assert(std::is_base_of<CRTP<decltype(this)>,decltype(‌​this)>::value) in the derived classes – Woodcraft

@MooingDuck that line does always fail for every of the three class subtypes. – Lynnlynna

@nyarlathotep108, It'll need remove_reference_t<decltype(*this)>. – Hargreaves

Answer 3

A bit late to the party, but we encountered the same issue and went with a solution similar to Nir Friedman's answer except that we made the base destructor private rather than the constructor.

The main difference is that it allows forwarding the derived classes constructors to the base template.

Specifically:

template<typename T>
struct CRTP
{
    explicit CRTP(/* various arguments */) { /* ... */ }
    // ...

private: // enforce correct CRTP use
    ~CRTP() = default;
    friend T;
};

struct Good : CRTP<Good>
{
    using CRTP::CRTP; // will not compile if CRTP's ctor(s) is/are private
    // ...
};

struct Bad : CRTP<Good> // Compile error
{
    // ...
};

Other than this, we did not see a difference with our usage.

Live: https://godbolt.org/z/YhK38K9qo

Flavouring