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?

You can `static_cast` to derived classes. You can’t check at compile time what the runtime type will be. — molbdnilo, Jan 10 '18 at 16:42
@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). — chris, Jan 10 '18 at 16:43
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,decltype(this)>::value)`? — Mooing Duck, Jan 10 '18 at 16:44
@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 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 { ... };` — chris, Jan 10 '18 at 16:51
Both `A` and `C` are derived from `CRTP`, so a `static_cast` is allowed from `CRTP ` to either of them. A cast to an unrelated (or ambiguous) type, e.g. `B` would require a diagnostic, though. — Arne Vogel, Jan 10 '18 at 18:41

score 10 · Accepted Answer · answered Jan 10 '18 at 17:24

10

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.

answered Jan 10 '18 at 17:24

Nir Friedman

17,108
2
44
72

Doesn't this allow one to do something silly like `void MyBadClass::memberFunction() {CRTP x; x.do_it();}`? – Joshua Green Jan 11 '18 at 02:05
@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. – Nir Friedman Jan 11 '18 at 15:59
That's fair. I thought I had a solution to this, but I'm struggling to remember it at the moment. – Joshua Green Jan 12 '18 at 02:07
@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. – Nir Friedman Jan 12 '18 at 05:09
It does not compile (even in your link). – Fan Aug 06 '18 at 22:03
@Fan it's supposed to not compile, suggest rereading the question and answer. – Nir Friedman Aug 07 '18 at 02:09
I know why it does not compile on my side. I have `Abstract : CRTP` and then `Concrete : Abstract`. But because Abstract is not a friend of CRTP it does not compile. Is there a way to solve this? – Fan Aug 07 '18 at 15:19
1

@Fan yes, use the pattern correctly: `Abstract : CRTP>`. That said, there is no reason to have an "abstract" class in C++ static polymorphism. – Nir Friedman Aug 07 '18 at 19:04

R Sahu · Answer 2 · 2018-01-10T20:20:11.973

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.

edited Jan 10 '18 at 20:20

answered Jan 10 '18 at 16:40

R Sahu

204,454
14
159
270

"How can I have a compile time check that could help from this type of errors?" is really the key part of the question – Mooing Duck Jan 10 '18 at 16:43
@MooingDuck, I copied the solution from your comment. Hope you don't mind. – R Sahu Jan 10 '18 at 16:46
1

I wouldn't mind, but [chris](https://stackoverflow.com/users/962089/chris) observed that [it's wrong](https://stackoverflow.com/questions/48192075/has-crtp-no-compile-time-check/48192163?noredirect=1#comment83364793_48192075) – Mooing Duck Jan 10 '18 at 16:48
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. – nyarlathotep108 Jan 10 '18 at 16:50
@nyarlathotep108: Try `static_assert(std::is_base_of,decltype(‌this)>::value)` in the derived classes – Mooing Duck Jan 10 '18 at 16:53
@MooingDuck that line does always fail for every of the three class subtypes. – nyarlathotep108 Jan 10 '18 at 16:58
@nyarlathotep108, It'll need `remove_reference_t`. – chris Jan 10 '18 at 17:25

Has CRTP no compile time check?

2 Answers2