Why downcast and then assign to base-class in C++?

Question

I have stumbled upon the following code structure and I'm wondering whether this is intentional or just poor understanding of casting mechanisms:

struct AbstractBase{ 
    virtual void doThis(){
    //Basic implementation here.
    };
    virtual void doThat()=0;
};

struct DerivedA: public AbstractBase{ 
    virtual void doThis(){
    //Other implementation here.
    };
    virtual void doThat(){
    // some stuff here.
    };
};
// More derived classes with similar structure....

// Dubious stuff happening here:
void strangeStuff(AbstractBase* pAbstract, int switcher){
   AbstractBase* a = NULL;
   switch(switcher){
       case TYPE_DERIVED_A: 
                // why would someone use the abstract base pointer here???
                a = dynamic_cast<DerivedA*>(pAbstract);
                a->doThis();
                a->doThat();
                break;
       // similar case statement with other derived classes...
   }
}

// "main"
DerivedA* pDerivedA = new DerivedA;
strangeStuff( pDerivedA, TYPE_DERIVED_A );

My guess is, that this dynamic_cast statement is just the result of poor understanding and very bad programming style in general (the whole way the code works, just feels painful to me) and that it doesn't cause any change in behaviour for this specific use case.

However, since I'm not an expert on casting, I'd like to know whether there are any subtle side-effects that I'm not aware of.

Answer 1

Blockquote [C++11: 5.2.7/9]: The value of a failed cast to pointer type is the null pointer value of the required result type.

The dynamic_cast can return NULL if the type was wrong, making the following lines crash. Hence, this can be either 1. an attempt to make (logical) errors more explicit, or 2. some sort of in-code documentation.

So while it doesn't look like the best design, it is not exactly true that the cast has no effect whatsoever.

Answer 2

My guess would be that the coder screwed up.

A second guess would be that you skipped a check for a being null in your simplification.

A third, and highly unlikely possibility, is that the coder was exploiting undefined behavior to optimize.

With this code:

            a = dynamic_cast<DerivedA*>(pAbstract);
            a->doThis();

if a is not of type DerivedA* (or more derived), the a->doThis() is undefined behavior. And if it is of type DerivedA*, then the dynamic_cast does absolutely nothing (guaranteed).

A compiler can, in theory, optimize out any other possibility away, even if you did not change the type of a, and remain conforming behavior. Even if someone later checks if a is null, the execution of undefined behavior on the very next line means that the compiler is free not to set a to null on the dynamic_cast line.

I would doubt that a given compiler would do this, but I could be wrong.

There are compilers that detect certain paths cause undefined behavior (in the future), eliminate such possibilities from happening backwards in execution to the point where the undefined behavior would have been set in motion, and then "know" that the code in question cannot be in the state that would trigger undefined behavior. It can then use this knowledge to optimize the code in question.

Here is an example:

std::string foo( unsigned int x ) {
  std::string r;
  if (x == (unsigned)-1)) {
    r = "hello ";
  }
  int y = x;
  std::stringstream ss;
  ss << y;
  r += ss.str();
  return r;
}

The compiler can see the y=x line above. If x would overflow an int, then the conversion y=x is undefined behavior. It happens regardless of the result of the first branch.

In short, if the first branch runs, undefined behavior would result. And undefined behavior can do anything, including time travel -- it can go back in time and prevent that branch from being taken.

So the

  if (x == (unsigned)-1)) {
    r = "hello ";
  }

branch can be eliminated by the optimizer, legally in C++.

While the above is just a toy case, gcc does optimizations very much like this. There is a flag to tell it not do.

(unsigned -1 is defined behavior, but overflowing an int is not, in C++. In practice, this is because there are platforms in which signed int overflow causes problems, and C++ doesn't want to impose extra costs on them to make a conforming implementation.)

Answer 3

dynamic_cast will confirm that the dynamic type does match the type indicated by the switcher variable, making the code slightly less dangerous. However, it will give a null pointer in the case of a mismatch, and the code neglects to check for that.

But it seems more likely that the author didn't really understand the use of virtual functions (for uniform treatment of polymorphic types) and RTTI (for the rarer cases where you need to distinguish between types), and attempted to invent their own form of manual, error-prone type identification.