Beware of Implicit Type Casting

CodeProject

Type casting consists of converting an expression of a given type into another type. It can be done by explicitly telling the compiler which type the expression must be converted to, for instance:

float x = 3.14;
int i = int(x);  // i is assigned to 3.
A* a = foo();
B* b = static_cast<B*>(a);
C* c = reinterpret_cast<C*>(b);

It can also be done implicitly by letting the compiler decide which type conversion is appropriate to successfully compile the source code. It follows type conversion rules, e.g., walk up the class hierarchy to find the implementation of a non-virtual method. This makes the code simpler to write and easier to read. However adding one’s own rules to the lot can lead to subtle bugs, whose cause is rooted in implicit type casting. I will illustrate with a simple example, inspired by a recent real case.

Let us say that you have a class A, and 100,000's lines of legacy code using pointers to A. The code looks something like this. Note how the code tests for NULL pointers (lines 17, 19, 21): pointers are implicitly converted to Boolean, which is perfectly correct.

class A;
typedef A* ptra;

class A {
public:
  int foo(ptra a, ptra b);
  int bar();
  int operator[](int i);
};

int A::foo(ptra a, ptra b) {
  if (a && b) {
    return (*a)[b->bar()];
  } else if (a && !b) {
    return a->bar();
  } else if (b) {
    return b->bar();
  } else {
    return 0;
  }
}

Later, the behavior of the type ptra was extended (for instance, to add a reference count). A class wrapping ptra was added to avoid disrupting the existing code and public APIs. It looked like this:

template <class T>
class Ptr {
public:
    Ptr() : p_(NULL) {}
    Ptr(T* p) : p_(p) {}

    T* get() const { return p_; }
    T* operator -> () const { return p_; }
    T& operator * () const { return *p_; }

    operator bool () const { return (p_ != NULL); }
private:
    T* p_;
};

typedef Ptr<A> ptra;

Note the type conversion of Ptr to bool defined in line 37. Thanks to it, the existing code that implicitly checks for NULL pointers does not need to be touched: the statement ‘if (a) {...}‘ will behave as before. However, the implicit type conversion has unexpected effects. Consider the following code:

typedef Ptr<int> PtrInt;
typedef Ptr<char> PtrChar;

int main() {
    int i1;
    int i2;
    PtrInt a1(&i1);
    PtrInt a2(&i2);

    PtrChar b2((char*)&i2);

    assert(a1.get() != a2.get());
    if (a1 != a2) {
        cout << "OK: a1 != a2\n";
    } else {
        cout << "WRONG: a1 != a2 failed.\n";
    }

    if (b2 == a2) {
        cout << "OK: b2 == a2\n";
    } else {
        cout << "WRONG: b2 == a2 failed.\n";
    }

    if (b2 != a1) {
        cout << "OK: b2 != a1\n";
    } else {
        cout << "WRONG: b2 != a1 failed.\n";
    }

    return 0;
}

It produces the following output:

ocoudert:~/src$ g++ sample.cc && a.out
WRONG: a1 != a2 failed.
OK: b2 == a2
WRONG: b2 != a1 failed.

Quick, which output lines are correct? The answer is: none of them.

Line 61 and 67 are problematic: they compare two objects templated with two different classes. They should not even compile! Line 55 goes against the intuition: both a1 and a2 do wrap different pointers, as asserted by line 54.

In line 55, lacking an explicit Ptr::operator== definition, both a1 and a2 are implicitly converted to a bool. Because a1 and a2 are non null, they are seen as true values, resulting in the ‘else’ branch to be executed. Thanks to the implicit promotion to bool, line 61 and 67 compile properly, but the result of such comparisons is irrelevant.

To fix the problem on line 55, we need to explicitly define comparison on Ptr.

template <class T>
bool operator == (const Ptr<T>& a, const Ptr<T>& b) { return a.get() == b.get(); }
template <class T>
bool operator != (const Ptr<T>& a, const Ptr<T>& b) { return a.get() != b.get(); }

This will produce:

ocoudert:~/src$ g++ sample.cc && a.out
OK: a1 != a2
OK: b2 == a2
WRONG: b2 != a1 failed.

For lines 61 and 67, we need to decide of the semantics when comparing two Ptr instantiated with different classes. Let’s say that the comparison should be on the type, not the pointer:

template <class T, class U>
bool operator == (const Ptr<T>& a, const Ptr<U>& b) { return false; }
template <class T, class U>
bool operator != (const Ptr<T>& a, const Ptr<U>& b) { return true; }

We now have:

ocoudert:~/src$ g++ sample.cc && a.out
OK: a1 != a2
WRONG: b2 == a2 failed.
OK: b2 != a1

If we choose to carry the comparison on the pointer itself, we would write:

template <class T, class U>
bool operator == (const Ptr<T>& a, const Ptr<U>& b) { return a.get() == (T*)b.get(); }
template <class T, class U>
bool operator != (const Ptr<T>& a, const Ptr<U>& b) { return a.get() != (T*)b.get(); }

And we get instead:

ocoudert:~/src$ g++ sample.cc && a.out
OK: a1 != a2
OK: b2 == a2
OK: b2 != a1

However, we are still not out of the woods. We need to consider all the Boolean operators. For example, we can still write ((a1 < a2) || (a2 < a1)), and that expression will evaluate to false. The complete approach should follow the safe bool idiom.

The conclusion: implicit type casting makes the code easier to write and more legible; it is also a great tool to work with legacy code one cannot afford to change. But this must be done very carefully. Unwanted implicit type casting may create more problems than they solve.