Question

So I have a variant class that I've recently added the ability to store pointer-to-member-function data. It uses the following code to accomplish that.

class Variant
{
    void* _value;

    template <typename T1>
    void Variant::ThisCall( T1* pThis )
    {
        typedef void(T1::* fptr)( );
        fptr a;
        int* iiVal = new (&a) int;
        *iiVal = *((int*)_value);
        (pThis->*a)( );
    }
};

// usage
Variant myfunc = &SomeClass::SomeMethod;
SomeClass* s = new SomeClass( );
myfunc.ThisCall( s );

The biggest thing I've worked around for this solution is that pointer-to-member functions cannot be cast to void*. So the assignment operator sort of does the inverse of this operation essentially. It takes the given data, masks it as an int pointer (if it is a pointer itself) and assigns the int pointer to the void*, which is perfectly legal.

So my question is this: Why do I feel like this is a horrible solution to the problem? I feel like this is such a big hack that there has to be some serious problems with it, but I've been so deep in this problem for a few days now that I can't see past it. Thanks!

[EDIT #1]

One commenter noted that this may not work with virtual methods. I have tested using the following code and it seems to check out.

class ImplA : public Base
{
public:
    virtual void Print( )
    {
        cout << "ImplA print\n";
    }
};

class ImplB : public Base
{
public:
    virtual void Print( )
    {
        cout << "ImplB print\n";
    }
};

class ImplC : public ImplA
{
public:
    virtual void Print( )
    {
        cout << "ImplC print\n";
    }
};

// usage
Variant x = &Base::Print;
auto b = new ImplA; // replace ImplA with ImplB or ImplC and it works as expected
x.ThisCall( b );

For some additional info, I'm using VS2010 as my compiler. Thanks!

[EDIT #2]

To provide context, I have been working on this variant class for a short while now and am trying to make it support anything you could throw at it. In doing this, I thought about function pointers and member function pointers. Then I came up with this and wondered how solid this solution actually is. The casting and syntax was the first red flag for me, but I figured, because of the variance of data it holds, that just comes with the territory. But I'm still not convinced this is how this should work.

Was it helpful?

Solution

Ignoring the aliasing violation, which itself makes your code illegal, what you're doing is equivalent to this:

    typedef void(T1::* fptr)( );
    fptr a;
    memcpy(&a, _value, sizeof(int));
    (pThis->*a)( );

It should be obvious why this is non-portable; there is no guarantee that fptr has the same size as int, so you are likely to either partially initialise its storage or overflow it.

This would be legal if you replace sizeof(int) with sizeof(fptr), and ensure that the storage that _value points to is large enough to contain a fptr. However, you should still use memcpy instead of aliasing; memcpy is guaranteed to work (3.9p2) while aliasing can result in hard-to-detect bugs that typically show up or change behaviour under optimisation.

OTHER TIPS

Like mentioned in comments this code is not very portable and safe. If you are just storing pointers to function I would suggest using std::function or boost::function wrappers:

template <typename T>
class Variant {
    std::function<void(T*)> fun;
public:
    Variant(void (T:: *ptr)() ) : fun(ptr) {
    }

    void ThisCall(T* pThis) {
        fun(pThis);
    }
};

Variant<SomeClass> myfunc = &SomeClass::SomeMethod;
SomeClass* s = new SomeClass( );
myfunc.ThisCall( s );

But if you really want to store anything why not just use boost::any library?

class VariantWithAny {
    boost::any val;
public:
    VariantWithAny() {}

    VariantWithAny(const boost::any& val) : val(val) {}

    VariantWithAny& operator=(const boost::any& val) {
        this->val = val;
        return *this;
    }

    template <typename T>
    void ThisCall(T* pThis) {
        typedef void (T::* fptr)();
        fptr a = boost::any_cast<fptr>(val);
        (pThis->*a)( );
    }
};

VariantWithAny myfunc2(&SomeClass::SomeMethod1);
myfunc2 = &SomeClass::SomeMethod2;
SomeClass* s2 = new SomeClass( );
myfunc2.ThisCall( s2 );

boost::any_cast is safe and will throw an exception (boost::bad_any_cast) if the types do not match.

[EDIT]: The trick that boost::any uses is to store the value in a template holder class that inherits from pure virtual placeholder. That is how it can hold almost any value and does not have to cast it to (void*). Check out the implementation - it is a very small file.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top