Prevent user from deriving from incorrect CRTP base
Asked Answered
R

6

25

I cannot think about a proper question title to describe the problem. Hopefully the details below explains my problem clear.

Consider the following code

#include <iostream>

template <typename Derived>
class Base
{
    public :

    void call ()
    {
        static_cast<Derived *>(this)->call_impl();
    }
};

class D1 : public Base<D1>
{
    public :

    void call_impl ()
    {
        data_ = 100;
        std::cout << data_ << std::endl;
    }

    private :

    int data_;
};

class D2 : public Base<D1> // This is wrong by intension
{
    public :

    void call_impl ()
    {
        std::cout << data_ << std::endl;
    }

    private :

    int data_;
};

int main ()
{
    D2 d2;
    d2.call_impl();
    d2.call();
    d2.call_impl();
}

It will compile and run though the definition of D2 is intentionally wrong. The first call d2.call_impl() will output some random bits which is expected as D2::data_ was not initialized. The second and third calls will all output 100 for the data_.

I understand why it will compile and run, correct me if I am wrong.

When we make the call d2.call(), the call is resolved to Base<D1>::call, and that will cast this to D1 and call D1::call_impl. Because D1 is indeed derived form Base<D1>, so the cast is fine at compile time.

At run time, after the cast, this, while it is truly a D2 object is treated as if it is D1, and the call to D1::call_impl will modified the memory bits that are supposed to be D1::data_, and output. In this case, these bits happened to be where D2::data_ are. I think the second d2.call_impl() shall also be undefined behavior depending on the C++ implementation.

The point is, this code, while intensionally wrong, will give no sign of error to the user. What I am really doing in my project is that I have a CRTP base class which acts like a dispatch engine. Another class in the library access the CRTP base class' interface, say call, and call will dispatch to call_dispatch which can be base class default implementation or derived class implementation. These all will work fine if the user defined derived class, say D, is indeed derived from Base<D>. It will raise compile time error if it is derived from Base<Unrelated> where Unrelated is not derived from Base<Unrelated>. But it will not prevent user write code like above.

The user use the library by deriving from the base CRTP class and providing some implementation details. There are certainly other design alternatives that can avoid the problem of incorrect use as above (for example an abstract base class). But let's put them aside for now and just believe me that I need this design because of some reason.

So my question is that, is there any way that I can prevent the user from writing incorrect derived class as see above. That is, if user write an derived implementation class, say D, but he derived it from Base<OtherD>, then a compile time error shall be raised.

One solution is use dynamic_cast. However, that is expansive and even when it works it is a run-time error.

Rolan answered 27/6, 2012 at 11:8 Comment(4)
Short answer: no. dynamic_cast may be expensive, but it is cheaper than trying to fix your users.Villainage
At least the failing dynamic_cast will be found faster in the unit tests. You can't have the compiler protect you from every possible typo, like writing i + 1 when meaning i - 1. This is similar.Meador
possible duplicate of How to avoid errors while using CRTP?Market
Duplicate? https://mcmap.net/q/427878/-how-to-avoid-errors-while-using-crtp/946850Foretopmast
D
36

1) make all constructors of Base private (if there are no constructors, add one)

2) declare Derived template parameter as friend of Base

template <class Derived>
class Base
{
private:

  Base(){}; // prevent undesirable inheritance making ctor private
  friend  Derived; // allow inheritance for Derived

public :

  void call ()
  {
      static_cast<Derived *>(this)->call_impl();
  }
};

After this it would be impossible to create any instances of the wrong inherited D2.

Dekameter answered 28/6, 2012 at 8:49 Comment(6)
I think this shall work! Why these really smart thing always turns out to be really simple! This looks a lot like a Barton–Nackman trickRolan
@Yan Zhou:I tried it, it seems to work :) I also just found a related question on SO: #5908231 and answered it too, but, perhaps, there are some pitfalls as the question appears more general (not about wrong inheritance only).Dekameter
@user396672: The only pitfall I can see is that it is ill-formed prior to C++11. The standard explicitly disallowed having a class template declare a template type-parameter as a friend.Nucleotidase
The problem with such approach is that it disallows more complex scenarios where Derived is not an immediate descendant of Base.Creditor
You can even mark Base private ctor as default, is more typing but it also works :)Beatitude
We use a private dtor instead of a private ctor, as explained here.Segment
P
5

If you have C++11 available, you can use static_assert (if not, I am sure you can emulate these things with boost). You could assert for e.g. is_convertible<Derived*,Base*> or is_base_of<Base,Derived>.

This all takes place in Base, and all it ever has is the information of Derived. It will never have a chance to see if the calling context is from a D2 or D1, as this makes no difference, since Base<D1> is instantiated once, in one specific way, no matter whether it was instantiated by D1 or D2 deriving from it (or by the user explicitly instantiating it).

Since you do not want to (understandably, since it has sometimes significant runtime cost and memory overhead) use dynamic_cast, try to use something often called "poly cast" (boost has its own variant too):

template<class R, class T>
R poly_cast( T& t )
{
#ifndef NDEBUG
        (void)dynamic_cast<R>(t);
#endif
        return static_cast<R>(t);
}

This way in your debug/test builds the error gets detected. While not a 100% guarantee, in practice this often catches all the mistakes people make.

Peugia answered 27/6, 2012 at 11:35 Comment(6)
I had the same Idea,but this is not as easy as it seems. If you use it in the body of Base like this static_assert( is_base_of<Base<Derived>, Derived>::value, "...") this will not work, because if the template get's instantiated with D1 this will become static_assert( is_base_of< Base< D1 >, D1 >::value, "...") which does not trigger the assert (after all D1 is derived from Base< D1 >). The only errors this catches are the ones where derives from Base< D3 > where D3 does not itself inherit from the correct Base. These errors are however already captured by the static_cast.Prospector
@LiKao: ah, now I see what you mean, let me add that to the answer.Peugia
Thanks for the poly cast hint. I just come up with a very similar idea. I assert the dynamic_cast in debug mode and then do the static_cast. The template solution is clearly also very elegant.Rolan
+1 for the poly cast. This type of cast (or the similar way such as assert_cast) are so nice they should be part of the standard IMHO.Prospector
This solution requires Base be to be a polymorphic type. With the original posted code, where Base is not polymorphic, the dynamic cast itself will fail to compile.Nucleotidase
@twsansbury: I was assuming that this is obvious and that this can easily be done by wrapping virtual dtor into the same NDEBUG in Base...Peugia
H
2

General point: Templates are not protected from being instantiated with wrong params. This is well known issue. It is not recommended to spend time in trying to fix this. The number or ways how templates can be abused is endless. In your particular case you might invent something. Later on you will modify your code and new ways of abusing will show up.

I know that C++11 has static assert that might help. I do not know full details.

Other point. Besides compiling errors there is static analysis. What you are asking for has some something with this. Analysis does not necessarily look for security flaws. It can ensure that there is no recusrion in the code. It can check that there is no derivatives from some class, you can pose restrictions on params of templates and functions, etc. This is all analysis. Such widely varying constrains cannot be supported by the compiler. I am not sure that this is the right way to go, just telling about this possibility.

p.s. Our company provides services in this area.

Hartzog answered 27/6, 2012 at 11:25 Comment(4)
I can't really find anything here that would answer his question, only promotion for your company...Peugia
Come on. I am saying that: 1. C++2003 is not giving good protection for wrong template instantiation. 2. C++11 might help. 3. Algorithmic analysis might help. Not necessarily with the help of our tools. Where am I saying that other tools will not work?Hartzog
He is asking /what/ helps, exactly. Telling that there is something out there that might help is as much as I guess he already figured outPeugia
My main advice is that looking for ways of protecting templates in C++2003 is not the right way to go. There are articles and maybe even books about this.Hartzog
S
1

If you can't count with C++11, you could try this trick:

  1. Add a static function in Base that returns a pointer to its specialied type:

    static Derived *derived( ) { return NULL; }

  2. Add a static check function template to base that takes a pointer:

    template< typename T > static bool check( T *derived_this ) { return ( derived_this == Base< Derived >::derived( ) ); }

  3. In your Dn constructors, call check( this ):

    check( this )

Now if you try to compile:

$ g++ -Wall check_inherit.cpp -o check_inherit
check_inherit.cpp: In instantiation of ‘static bool Base<Derived>::check(T*) [with T = D2; Derived = D1]’:
check_inherit.cpp:46:16:   required from here
check_inherit.cpp:19:62: error: comparison between distinct pointer types ‘D2*’ and ‘D1*’ lacks a cast                                                                                                                             
check_inherit.cpp: In static member function ‘static bool Base<Derived>::check(T*) [with T = D2; Derived = D1]’:                                                                                                                   
check_inherit.cpp:20:5: warning: control reaches end of non-void function [-Wreturn-type]                                                                                                                                          
Silverside answered 27/6, 2012 at 12:2 Comment(0)
P
1

In general I do not think there is a way to get this, which should not be considered outright ugly and reverts to the usage of evil features. Here is a summary of what would work and what would not.

  • Use of static_assert (either from C++11 or from boost) does not work, because a check in the definition of Base can only use the types Base<Derived> and Derived. So the following will look good, but fail:

    template <typename Derived>
    class Base
    {
       public :
    
       void call ()
       {
          static_assert( sizeof( Derived ) != 0 && std::is_base_of< Base< Derived >, Derived >::value, "Missuse of CRTP" );
          static_cast<Derived *>(this)->call_impl();
       }
    };
    

In case you try to declare D2 as class D2 : Base< D1 > the static assert will not catch this, since D1 actualy is derived from Base< D1 > and the static assert is completely valid. If you however derive from Base< D3 > where D3 is any class not deriving from Base< D3 > both the static_assert as well as the static_cast will trigger compilation errors, so this is absolutely useless.

Since the type D2 you would need to check in the code of Base is never passed to the template the only way to use static_assert would be to move it after the declarations of D2 which would require the same person who implemented D2 to check, which again is useless.

One way to get around this would be by adding a macro, but this would beget nothing but pure ugliness:

#define MAKE_DISPATCHABLE_BEGIN( DeRiVeD ) \
   class DeRiVeD : Base< DeRiVed > {
#define MAKE_DISPATCHABLE_END( DeRiVeD )
    }; \
    static_assert( is_base_of< Base< Derived >, Derived >::value, "Error" );

This only gains ugliness, and the static_assert is again superflous, because the template makes sure that the types alway match. So no gain here.

  • Best option: Forget about all this and use dynamic_cast which was explicitely meant for this scenario. If you need this more often it would probably make sense to implement your own asserted_cast (there is an article on Dr. Jobbs on this), which automatically triggers a failed assert when the dynamic_cast fails.
Prospector answered 27/6, 2012 at 12:12 Comment(0)
N
1

There is no way to prevent the user from writing incorrect derived classes; however, there are ways to prevent your code from invoking classes with unexpected hierarchies. If there are points at which the user is passing Derived to the library functions, consider having those library functions perform a static_cast to the expected derived type. For example:

template < typename Derived >
void safe_call( Derived& t )
{
  static_cast< Base< Derived >& >( t ).call();
}

Or if there are multiple levels of hierarchy, consider the following:

template < typename Derived,
           typename BaseArg >
void safe_call_helper( Derived& d,
                       Base< BaseArg >& b )
{
   // Verify that Derived does inherit from BaseArg.
   static_cast< BaseArg& >( d ).call();
}

template < typename T >
void safe_call( T& t )
{
  safe_call_helper( t, t );  
}

In both of thse cases, safe_call( d1 ) will compile while safe_call( d2 ) will fail to compile. The compiler error may not be as explicit as one would like for the user, so it may be worthwhile to consider static asserts.

Nucleotidase answered 27/6, 2012 at 12:14 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.