CODEWITHSUNDEEP
c++casting
sharptooth
sharptooth

Reputation: 170489

Any reason to prefer static_cast over a chain of implicit conversions?

Suppose I have a class implementing several interfaces

class CMyClass : public IInterface1, public IInterface2 { };

and in a member function of that class I need to obtain a void* pointer to one of those interfaces (typical situation in IUnknown::QueryInterface().

The typical solution is to use a static_cast to achieve pointer adjustment:

void* pointer = static_cast<IInterface2*>( this );

and it is safe in this case if there's no known class inherited from CMyClass. But if such class exists:

class CDerivedClass : public CUnrelatedClass, public CMyClass {};

and I accidentially do

void* pointer = static_cast<CDerivedClass*>( this );

and this is actually a pointer to CMyClass instance the compiler won't catch me and the program might run into undefined behavior later - static_cast becomes unsafe.

The suggested solution is to use implicit conversion:

IInterface2* interfacePointer = this;
void* pointer = interfacePointer;

Looks like this will solve both problems - pointer adjustment and risk of invalid downcast.

Are there any problems in the second solution? What could be the reasons to prefer the first one?

Upvotes: 4

Views: 750

Answers (5)

Sjoerd
Sjoerd

Reputation: 6875

You could use this template:

template<class T, class U> T* up_cast(U* p) { return p; }

usage:

struct B {};
struct C : B {};

int main()
{
  C c;

  void* b = up_cast<B>(&c);
}

Note that the '*' is implicit. If you prefer up_cast<B*>, adjust the template accordingly.

Upvotes: 3

wilx
wilx

Reputation: 18228

If you are afraid of doing something by accident with the static_cast then I suggest that you wrap the casting/interface pointer obtaining into some template function, e.g. like this:

template <typename Interface, typename SourceClass>
void set_if_pointer (void * & p, SourceClass * c)
{
  Interface * ifptr = c;
  p = ifptr;
}

Alternatively, use dynamic_cast and check for the NULL pointer value.

template <typename Interface, typename SourceClass>
void set_if_pointer (void * & p, SourceClass * c)
{
  p = dynamic_cast<Interface *>(c);
}

Upvotes: 1

Tony Delroy
Tony Delroy

Reputation: 106076

Your analysis looks sound to me. The reason not to use your implicit approach are not compelling:

  • slightly more verbose
  • leaves variables hanging around
  • static_cast<> is arguably more common, therefore more likely to be obvious to other developers, searched for etc.
  • in many cases even the declarations of derived classes won't appear before the definition of the base class functions, so there's no potential for this type of error

Upvotes: 1

Simone
Simone

Reputation: 11797

I can't see any reason in not using the latter solution other than the fact that, if somebody else is reading your code it won't communicate immediatly why you are using such a convoluted statement ("why isn't he just using a static_cast?!?"), so it would be better to comment it or make the intent very clear.

Upvotes: 2

Chris Becke
Chris Becke

Reputation: 36016

Assigning to void* is always unsafe. Whichever way you write it you can mess up - assuming that the user is trying to QI for Interface1, then neither of the following will be a warning or error:

Interface2* interfacePointer = this;
void* pointer = interfacePointer;

or

void* pointer = static_cast<Interface2*>( this );

Given the tiny risk of accidentally using a static_cast to up cast, in a file that most likely wont even have access to the definition of the derived class, I see a lot of extra effort for very little actual safety.

Upvotes: 2

Related Questions