How I refactor virtual functions


As with many development projects, I had to refactor some code in KMDF. This refactor involved changing the signature of a virtual function to take additional parameters. The problem I faced is that C++ makes no distinction between declaring a new virtual function and a virtual function which overrides a base class (C# does not have this problem because you are required to use the override keyword when overriding a base class virtual). The problem for me was finding all of the derivations across the entire code base.Before I explain into how I found all of the derivations, I want to explain the issue in more detail. Let’s take the following classes:

    class Base { public: virtual void Foo(); }

class Derived : public Base { public: virtual void Foo(); }

Derived::Foo() is a override of Base::Foo() and will be invoked correctly if using a Derived object pointer as a Base object pointer. The debugger confirms this
    0:000> dt der
Local var @ 0xcfe60 Type Derived
+0x000 __VFN_table : 0x008e10cc

0:000> dps 0x008e10cc l1
008e10cc 008e11f0 testsize!Derived::Foo

But let’s say I want to change the signature of Foo to take a Bar* and forgot to change Derived
    class Base { public: virtual void Foo(Bar* PBar); }

class Derived : public Base { public: virtual void Foo(); }

Now Derived::Foo() is not a derivation of Base::Foo, but it’s new and very own virtual function (e.g. the vtable for the Derived object will have 2 slots instead of just one) as we can see from the debuggerA
    0:000> dt der
Local var @ 0x13fd38 Type Derived
+0x000 __VFN_table : 0x001810cc

Exact matches:
0:000> dps 0x001810cc l2
001810cc 00181250 testsize!Base::Foo
001810d0 00181200 testsize!Derived::Foo

As you can see, the vtable accidentally grew when I changed Base::Foo’s signature and unfortunately the compiler doesn’t care. Even worse, the program will run and probably limp along with the error showing up as odd or unexplainable behavior. So, the challenge is to make the compiler find all of the derivations for you so you can fix them up. I use two techniques to make the compiler do the work for me.

The first technique is to change the return type of the function to refactor without changing the parameters themselves and then recompiling the project. This works because the rules for C++ name overloading allow for differences in the function’s parameters, but if the parameters are the same, you cannot differentiate on return type. The error will be emitted in the class’s implementation. So if you change Base::Foo to ULONG Foo();, the compiler will give you an error like this for every derived class which overrides Base::Foo()

    main.cpp(7) : error C2555: ‘Derived::Foo’: overriding virtual function return type differs and is not covariant from ‘Base::Foo’.”
After flagging each derived class, you can change the change the return type back and then change the parameters.

The second technique is to change the Base::Foo() to a pure virtual fuction in addition to changing the parameters. So, now Base::Foo() is declared as void Foo(Bar* PBar) =0;. Now for every class which is not an abstract base class itself, the compiler will emit an error wherever you try to instantiate the class, getting an error like this

    main.cpp(11) : error C2259: ‘Derived’ : cannot instantiate abstract class
The advantage of this technique is that you don’t have to make 2 passes over the code to find and then alter the derived classes. A problem with this technique is that you are only notified of the problem where you try to instantiate the class instead of wheree you implement or declare the class. Another problem is that if you are implementing a derivation in a class which itself is still an abstract base class, you push the compiler error into the second derived class, obscuring where the real error is occurring.

I personally prefer the first method because it is less error prone and I like to see compiler errors at the site of the implementation and not at the site of its use.

Comments (6)

  1. mygreenpaste says:

    Hi Doron,

    Thanks much for the blog – I enjoy reading it.  A few observations that may warrant clarification…

    1) >change the Base::Foo() to an abstract base class in addition to changing the parameters

    I suspect you probably meant something like "change Base to an abstract base class by making Base::Foo() a pure virtual function"…

    2) >where you try to instantiate the device

    Heh… Class here, it would seem… Doesn’t look like any devices today.

    -mgp

  2. doronh says:

    Uou are correct on both counts, corrections made in the post.  thx!

    d

  3. Niall Connaughton says:

    My kingdom for an override keyword…

  4. doronh says:

    Hear hear!  I would love it if there was a C++ version of the keyword (that was obviously opt in since it would break existing code).  I think I’ll poke around and see who I can ask in the compilers group for such a feature…

    d

  5. Yesterday I wrote about the two methods I use to refactor a virtual function and make sure that I find

  6. Jim Lyon says:

    Use __override, and PreFast will do exactly what you want.