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

Question

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?

Its interesting that `CMyClass` has knowledge of `CDerivedClass` here... Not impossible, not even a real sign of horrible design, but in the general case, `CMyClass` should not have any knowledge of its descendants. I can imagine having the definitions of both classes in headers and the translation unit where `CMyClass` methods are defined including both, more so in VS with its tendency to advocate precompiled headers... still, food for thought. — David Rodríguez - dribeas, Feb 10 '11 at 09:13

Sjoerd · Accepted Answer · 2011-02-10T14:44:26.790

3

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.

edited Feb 10 '11 at 14:44

answered Feb 10 '11 at 08:55

Sjoerd

6,837
31
44

That's called "upcast" - base classes are usually drawn at the top, derived at the bottom. – sharptooth Feb 10 '11 at 13:48

score 2 · Answer 2 · answered Feb 10 '11 at 08:43

2

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.

answered Feb 10 '11 at 08:43

Chris Becke

34,244
12
79
148

"a lot of extra effort" for two lines instead of one seems a little exaggerated claim to me. – Simone Feb 10 '11 at 08:49
I think you should read effort as effort to read, rather than effort to write. – daramarak Feb 10 '11 at 13:47

score 2 · Answer 3 · answered Feb 10 '11 at 08:48

2

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.

answered Feb 10 '11 at 08:48

Simone

11,655
1
30
43

Using one of the template suggestions in the other answers, you can achieve both safety and clearity. – Sjoerd Feb 10 '11 at 09:13
1

I feel it stills missing something, but at least it's more concise. – Simone Feb 10 '11 at 09:29

score 1 · Answer 4 · answered Feb 10 '11 at 08:59

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

score 1 · Answer 5 · answered Feb 10 '11 at 09:00

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);
}

`dynamic_cast` is waste of time (about 2 thousand cycles) and won't spot a dumb mistake until execution passes through that code. Yes, it's better than nothing, but compile-time checks are usually better than equivalent runtime checks. — sharptooth, Feb 10 '11 at 13:49

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

5 Answers5

Linked