Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Prevent user from deriving from incorrect CRTP base

Tags:

c++

crtp

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.

like image 432
Yan Zhou Avatar asked Jun 27 '12 11:06

Yan Zhou


3 Answers

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.

like image 56
user396672 Avatar answered Nov 07 '22 14:11

user396672


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.

like image 34
PlasmaHH Avatar answered Nov 07 '22 13:11

PlasmaHH


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.

like image 30
Kirill Kobelev Avatar answered Nov 07 '22 14:11

Kirill Kobelev