GotW #5 Solution: Overriding Virtual Functions

Virtual functions are a pretty basic feature, but they occasionally harbor subtleties that trap the unwary. If you can answer questions like this one, then you know virtual functions cold, and you’re less likely to waste a lot of time debugging problems like the ones illustrated below.

Problem

JG Question

1. What do the override and final keywords do? Why are they useful?

Guru Question

2. In your travels through the dusty corners of your company’s code archives, you come across the following program fragment written by an unknown programmer. The programmer seems to have been experimenting to see how some C++ features worked.

(a) What could be improved in the code’s correctness or style?

(b) What did the programmer probably expect the program to print, but what is the actual result?

class base {
public:
    virtual void f( int );
    virtual void f( double );
    virtual void g( int i = 10 );
};

void base::f( int ) {
    cout << "base::f(int)" << endl;
}

void base::f( double ) {
    cout << "base::f(double)" << endl;
}

void base::g( int i ) {
    cout << i << endl;
}

class derived: public base {
public:
    void f( complex<double> );
    void g( int i = 20 );
};

void derived::f( complex<double> ) {
    cout << "derived::f(complex)" << endl;
}

void derived::g( int i ) {
    cout << "derived::g() " << i << endl;
}

int main() {
    base    b;
    derived d;
    base*   pb = new derived;

    b.f(1.0);
    d.f(1.0);
    pb->f(1.0);

    b.g();
    d.g();
    pb->g();

    delete pb;
}

Solution

1. What do the override and final keywords do? Why are they useful?

These keywords give explicit control over virtual function overriding. Writing override declares the intent to override a base class virtual function. Writing final makes a virtual function no longer overrideable in further-derived classes, or a class no longer permitted to have further-derived classes.

They are useful because they let the programmer explicitly declare intent in a way the language can enforce at compile time. If you write override but there is no matching base class function, or you write final and a further-derived class tries to implicitly or explicitly override the function anyway, you get a compile-time error.

Of the two, by far the more commonly useful is override; uses for final are rarer.

2. (a) What could be improved in the code’s correctness or style?

First, let’s consider some style issues, and one real error:

1. The code uses explicit new, delete, and an owning *.

Avoid using owning raw pointers and explicit new and delete except in rare cases like when you’re writing the internal implementation details of a low-level data structure.

{
    base*   pb = new derived;

    ...

    delete pb;
}

Instead of new and base*, use make_unique and unique_ptr<base>.

{
    auto pb = unique_ptr<base>{ make_unique<derived>() };

    ...

} // automatic delete here

Guideline: Don’t use explicit new, delete, and owning * pointers, except in rare cases encapsulated inside the implementation of a low-level data structure.

However, that delete brings us to another issue unrelated to how we allocate and manage the lifetime of the object, namely:

2. base’s destructor should be virtual or protected.

class base {
public:
    virtual void f( int );
    virtual void f( double );
    virtual void g( int i = 10 );
};

This looks innocuous, but the writer of base forgot to make the destructor either virtual or protected. As it is, deleting via a pointer-to-base without a virtual destructor is evil, pure and simple, and corruption is the best thing you can hope for because the wrong destructor will get called, derived class members won’t be destroyed, and operator delete will be invoked with the wrong object size.

Guideline: Make base class destructors public and virtual, or protected and nonvirtual.

Exactly one of the following can be true for a polymorphic type:

  • Either destruction via a pointer to base is allowed, in which case the function has to be public and had better be virtual;
  • or else it isn’t, in which case the function has to be protected (private is not allowed because the derived destructor must be able to invoke the base destructor) and would naturally also be nonvirtual (when the derived destructor invokes the base destructor, it does so nonvirtually whether declared virtual or not).

Interlude

For the next few points, it’s important to differentiate three terms:

  • To overload a function f means to provide another function with the same name in the same scope but with different parameter types. When f is actually called, the compiler will try to pick the best match based on the actual parameters that are supplied.
  • To override a virtual function f means to provide another function with the same name and the same parameter types in a derived class.
  • To hide a function f that exists in an enclosing scope (base class, outer class, or namespace) means to provide another function with the same name in an inner scope (derived class, nested class, or namespace), which will hide the same function name in an enclosing scope.

3. derived::f is neither an override nor an overload.

    void derived::f( complex<double> )

derived does not overload the base::f functions, it hides them. This distinction is very important, because it means that base::f(int) and base::f(double) are not visible in the scope of derived.

If the author of derived intended to hide the base functions named f, then this is all right. Usually, however, the hiding is inadvertent and surprising, and the correct way to bring the names into the scope of derived is to write the using-declaration using base::f; inside derived.

Guideline: When providing a non-overridden function with the same name as an inherited function, be sure to bring the inherited functions into scope with a using-declaration if you don’t want to hide them.

4. derived::g overrides base::g but doesn’t say “override.”

    void g( int i = 20 )  /* override */

This function overrides the base function, so it should say override explicitly. This documents the intent, and lets the compiler tell you if you’re trying to override something that’s not virtual or you got the signature wrong by mistake.

Guideline: Always write override when you intend to override a virtual function.

5. derived::g overrides base::g but changes the default argument.

    void g( int i = 20 )

Changing the default argument is decidedly user-unfriendly. Unless you’re really out to confuse people, don’t change the default arguments of the inherited functions you override. Yes, this is legal C++, and yes, the result is well-defined; and no, don’t do it. Further below, we’ll see just how confusing this can be.

Guideline: Never change the default arguments of overridden inherited functions.

We could go one step further:

Guideline: Avoid default arguments on virtual functions in general.

Finally, public virtual functions are great when a class is acting as a pure abstract base class (ABC) that only specifies the virtual interface without implementations, like a C# or Java interface does.

Guideline: Prefer to have a class contain only public virtual functions, or no public virtual functions (other than the destructor which is special).

A pure abstract base class should have only public virtual functions. …

But when a class is both providing virtual functions and their implementations, consider the Non-Virtual Interface pattern (NVI) that makes the public interface and the virtual interface separate and distinct.

… For any other base class, prefer making public member functions non-virtual, and virtual member functions non-public; the former should have any default arguments and can be implemented in terms of the latter.

This cleanly separates the public interface from the derivation interface, lets each follow its natural form best suited for its distinct audience, and avoids having one function exist in tension from doing double duty with two responsibilities. Among other benefits, using NVI will often clarify your class’s design in important ways, including for example that the default arguments which matter to the caller therefore naturally belong on the public interface, not on the virtual interface. Following this pattern means that several classes of potential problems, including this one of virtuals with default arguments, just naturally don’t arise.

The C++ standard library follows NVI nearly universally, and other modern OO languages and environments have rediscovered this principle for their own library design guidelines, such as in the .NET Framework Design Guidelines.

2. (b) What did the programmer probably expect the program to print, but what is the actual result?

Now that we have those issues out of the way, let’s look at the mainline and see whether it does that the programmer intended:

int main() {
    base    b;
    derived d;
    base*   pb = new derived;

    b.f(1.0);

No problem. This first call invokes base::f( double ), as expected.

    d.f(1.0);

This calls derived::f( complex<double> ). Why? Well, remember that derived doesn’t declare using base::f; to bring the base functions named f into scope, and so clearly base::f( int ) and base::f( double ) can’t be called. They are not present in the same scope as derived::f( complex<double> ) so as to participate in overloading.

The programmer may have expected this to call base::f( double ), but in this case there won’t even be a compile error because fortunately(?) complex<double> provides an implicit conversion from double, and so the compiler interprets this call to mean derived::f( complex<double>(1.0) ).

    pb->f(1.0);

Interestingly, even though the base* pb is pointing to a derived object, this calls base::f( double ) because overload resolution is done on the static type (here base), not the dynamic type (here derived). You have a base pointer, you get the base interface.

For the same reason, the call pb->f(complex<double>(1.0)); would not compile, because there is no satisfactory function in the base interface.

    b.g();

This prints 10, because it simply invokes base::g( int ) whose parameter defaults to the value 10. No sweat.

    d.g();

This prints derived::g() 20, because it simply invokes derived::g( int ) whose parameter defaults to the value 20. Also no sweat.

    pb->g();

This prints derived::g() 10.

“Wait a minute!” you might protest. “What’s going on here?” This result may temporarily lock your mental brakes and bring you to a screeching halt until you realize that what the compiler has done is quite proper. (Although, of course, the programmer of derived ought to be taken out into the back parking lot and yelled at.) The thing to remember is that, like overloads, default parameters are taken from the static type (here base) of the object, hence the default value of 10 is taken. However, the function happens to be virtual, and so the function actually called is based on the dynamic type (here derived) of the object. Again, this can be avoided by avoiding default arguments on virtual functions, such as by following NVI and avoiding public virtual functions entirely.

    delete pb;
}

Finally, as noted, this shouldn’t be needed because you should be using unique_ptrs which do the cleanup for you, and base should have a virtual destructor so that destruction via any pointer to base is correct.

Acknowledgments

Thanks in particular to the following for their feedback to improve this article: litb1, KrzaQ, mttpd.

45 thoughts on “GotW #5 Solution: Overriding Virtual Functions

  1. Hmm… I like using final like this:

    class base {
     public:
      virtual ~base() = default;
      virtual void foo() = 0;
     protected:
      base() = default;
    }
    
    class derived final : public base {
     public:
      derived() = default;
      ~derived() = default;
      void foo() override { /* do foo */ }
    }
    
    and then use it like this:
    
    std::unique_ptr<base> bar = make_unique<derived>();
    bar->foo();
    

    Am I evil/misguided/leading my codebase down the primrose path?

  2. or else it isn’t, in which case the function has to be protected (private is not allowed because the derived destructor must be able to invoke the base destructor) and would naturally also be nonvirtual (when the derived destructor invokes the base destructor, it does so nonvirtually whether declared virtual or not).

    why it cant be public ? Doesnt placement new/delete require that(lets pretend that some crazy person wants to put polymorphic object in char[compile_time_max(sizeof (Base), sizeof (Der1),sizeof (Der2)…]
    :P

  3. @rhalbersma as an example of such a scoped guard see GotW 6b, part 6, option 1.

    {
        auto lock = std::unique_lock<mutex>{mutables};
        ::: // no further references to lock
        // lock destroyed; mutables released
    }
    
  4. The one thing I’ve found final useful for is for marking when functions in base classes have been renamed or are otherwise obsolete. For instance:

    class Foo
    {
    protected:
      virtual void bar() final {}; // Don't use this anymore. Use baz() instead.
      virtual void baz();
    };
    
  5. @Herb,
    I am talking with reference to what Rhalbersma said. Well the suggested way by Rhalbersma is something that is also adopted by modern languages and compilers. We just do x=y or as Herb said, by mistake z=y, it will create a variable of type same as y. Then compiler can use some smart, not at all sweating mechanism to figure out whether it is a correct assignment or not…
    We can take example of python, we do similar stuff in python, though its entirely differently implemented at language level but point is the ease we have with that..

    @Alok & Sil,

    I totally agree with what you are saying but remember Rome was not build in a day and so will your dream language. One thing I am sure you could feel the change. Some very smart people is constantly working hard for this change, probably we could see some major changes in C++14 or C++(yet to come) … :)

  6. @Ricardo Costa: thanks for the link. It’s actually quite apt for me. I do already have a decimal type created (using the decNumber C library) with the intention it could be cut over to built-in types eventually – so I do now see the possible applicability of “final” for library code.

  7. I hope I don’t :) I just want to point out (to Herb) that instead of the verbosity of
    auto pb = unique_ptr{ make_unique() };
    I find it better to have a guideline to use auto if there is no conversion during initialization, and write the type if there is. Or rephrasing:
    Use auto whenever you can, write the type only if you must.

    (“meh” is no english word, try to read it with some disapproval in your voice :) )

  8. @Róbert Dávid:

    “Meh”? Sorry, I am not native English speaker.

    I know that. You didn’t need to explain. :)

    I was just trying to simplify the code of Herb.

    auto pb = unique_ptr<Base>{ make_unique<Derived>() };
    
  9. @Fernando Pelliccioni:
    Meh. Just use the plain old

    std::unique_ptr<Base> pb = std::make_unique<Derived>();

    If you are using ‘auto’ for variables that have the same type as the initializer consistently, it will be obvious that there is a conversion here, and

    auto pd = std::make_unique<Derived>();

    has no conversion.

  10. Hi Herb,

    I think

    auto pb = std::unique_ptr<Base>{ std::make_unique<Derived>() };
    

    is too verbose. Unique is repeated…

    How about this library solution for C++14 or C++17 ?

    auto pb = make_unique_poly<base, derived>();
    

    Regards,
    Fernando.

  11. @Michael Urman: can you give a code example on how this scoped guard variable would occur?

  12. @Herb: “No, you still need a tag”

    I wonder, though, if there might be some specific contexts in which the ‘auto’ could be inferred if omitted, because you already know it’s a declaration. For example, within a ‘for-range’declaration’. This would allow the nicely concise syntax:

    for ( i : mycontainer) {}

  13. @rhalsbersma how would you differentiate between an unreferenced variable whose implicit declaration was due to a typo and an unreferenced variable whose lack of later reference is due to it being a scoped guard?

  14. Herb,

    In the guideline:

    Guideline: Prefer to have a class contain only public virtual functions, or no public virtual functions (other than the destructor which is special).

    It is unclear that protected virtual functions are possible and useful (protected interface) common in streams and a design pattern itself. Now, “no public virtual functions” can cover this usage of virtual functions, but it’s subtle.

    When you say:

    “A pure abstract base class should have only public virtual functions. …”

    Section 10.4 [class.abstract] only goes as far as defining an abstract class (neither pure nor “impure”) as a class with at least one pure virtual function. Since you can’t instantiate them, it follows that they can only be used as base classes. I think the sentence could be clearer if it were reworded as:

    “An abstract class with only pure virtual functions should have those functions be public.”

    This leaves open cases where you want an abstract class to have a public interface where it can control pre/post conditions and then call a protected virtual function on a derived class (I use this technique extensively in large telephony frameworks in C++ and C#):

    class Call
    {
      public:
        Drop()  { 
          // check preconditions, acquire locks, etc.,
          OnDrop();
         // check postconditions, release locks, etc.
        }
    
      protected:
        virtual void OnDrop() = 0;
    ...
    };
    
    class MyCall : public Call
    {
    ...
    protected:
      override void OnDrop() { ...}
    };
    

    Regards,

    –Javier Estrada

  15. @Alok I tend to agree with you that c++ can be extremely hard to understand and to program correctly and as expected without a very sizeable time investment. I also wonder how and if it could be made even simpler and safer than the progresses in 11.

    On the other hand isn’t it because it is used to design and architect a solution? i.e. if not in the C++ language itself there is still hard thinking to do and to implement for non trivial solutions, notably for modules or libraries, and this hard thinking has to be translated somehow, in guidelines or coding patterns or APIs for example, that will also have to be understood, implemented, checked and that will probably have pitfalls and tricky parts.

    Is auto pb = unique_ptr{ make_unique() }; a trivial statement? No but should it be used very often in a real life scenario? Probably not.

  16. C++ really needs fixing.

    While it makes sense to use things from the standard library when it provides one, the amount of thought process that needs to go into C++ programming (whether writing C++ standard library or own code) only shows the weaknesses of the language that blocks the programmer from going into higher level reasoning fast enough. I do not believe that the programmer (of own code or the library) has to think this hard (were the language design optimal) to express some simple concepts like this.

    Just look at this: “auto pb = unique_ptr{ make_unique() };”, and oh, this is after use of the auto keyword. There are things in C++ that need to be a part of the language itself which are a part of the library instead, and vice versa.

    I am waiting for the day when some other language like Go or Julia takes over.

  17. @rhalbersma
    auto would make a difference since you wouldnt write it before assignments.

    Also what about shadowing a variable?

  18. @Herb: I’m not really convinced by the off-by-one keyboard error. If you never reuse that variable z again, the compiler would warn (and give an error with warnings-as-errors) about an unused variable, and if you would use z again, then it might indeed silently compile and do the wrong thing, but auto wouldn’t make a difference. Can you give an example of any real compiler ambiguity?

  19. @rhalbersma: No, you still need a tag, just as Pascal uses “var” and other languages use other tags. Otherwise, it’s ambiguous or worse. For example, if “x = y;” could be a variable declaration, what if you mistyped x with an off-by-one keyboard error and typed “z = y;” instead? Then anytime you misspelled a variable on the left of an assignment the compiler would happily introduce a new variable and silently compile. Seems like a large trap. Variable declarations do need to be tagged, and four characters is very minimalistic… and unlike Pascal you can declare them anywhere in a scope.

  20. @Herb If **every** initialization is written in the form

    auto x = Type { expr }; 

    , isn’t that the same as saying that the auto keyword itself could be left out altogether? What parsing ambiguities could arise if it were? The compiler would only have to check if the lhs of any = token had been seen in the same scope before. Or are there some name lookup issues that would interfere with this?

  21. Yuck! That looks messed up, seems I’m too used to SO. Are there in-line code tags for comments like in the blog post itself?

  22. On the

    pb->f(1.0);

    you stress that overload resolution is done on the static type. I think the more surprising part for at least some people will be that having declared

    f

    virtual has no effect on the result because as you stated earlier

    f

    is not overridden but only hidden in

    derived

    , so the virtual function dispatch will still call

    base::f(double)

    .

  23. @Simon, @Arthur, and @Herb: You are all correct that I missed the slight change in semantics.

    For the learning purposes, then, I like Herb’s modified line, because it is more explicit about what is happening as well as a) reinforcing that auto should be used for things like this, and b) points out that a conversion is happening. I admit my eyes glazed over the first template specification, assuming a compatible result with make_unique.

    If I was actually reviewing a similar line in code, I’d expect auto to be used (or possibly to be assigned to a class member variable, which opens up different typing considerations).

  24. Guideline: Avoid default arguments on virtual functions in general.

    This needs to be more than just a guideline. It needs to be a _rule_.

    It’s just never a good idea. This is why function overloading exists.

  25. @GeoffW: Re “uses of final are rarer” – well, they sort of are. I don’t know of many, and during standardization Bjarne repeatedly asked for examples of problems it solved and patterns where it should be used, and I don’t recall any major ones that stood out. The only one I know of offhand is that if you’re defining a library module (which isn’t a Standard concept yet) then making leaf classes final can give the compiler more information to devirtualize calls because of knowing code outside the library won’t further derive, but I’m not sure how important that is these days in the presence of whole program optimization including aggressive devirtualization.

    @Johan: Okay, you guys are convincing me that maybe I’m worrying too much about acceptance of ‘auto everywhere.’ The reason I worried is because it’s a lightning rod issue that takes an amazing amount of energy in most C++11 overview sessions. Even on this blog when I announced I was restarting GotW and XC++, NEARLY EVERY comment on Reddit was about auto, which was mentioned only once in passing deep in the middle of the post. That’s all they could talk about. So I was leaning toward easing people into it, with a view of “you really need to get used to using auto frequently or at least consider it” while trying to avoid a brusque “this is the new way, get over it”… but okay, I’ve gone ahead with this change. Thanks for the feedback.

  26. I vote for

    auto pb = unique_ptr<base>{ make_unique<derived>() };

    I also like the improvement in both being conformed to the style of using auto as much as possible and seeing, in a more obvious way, the “conversion” from derived to base.
    Besides, since you are rewritting all GotW with the C++14 style, you shouldn’t be afraid by being unusual to most people’s eyes (i.e., people familiar with pre-C++14 style). On the contrary, not using auto to declare a variable seems unusual to me now.

  27. I don’t mean to be picky (well, maybe I do), but is “uses for final are rarer” the most that can be said for “final”? On seeing the original JG question I had sort of hoped for more (the question did ask: “Why are they useful?”). Maybe something about the sort of design situations that it was intended to aid – it must have been introduced into the standard for a reason.

  28. When you are using NVI for a base class A with a function foo(), and a sub class B wants to call foo() on itself, does B call the public or private version of foo()? It seems to me that the answer to that depends on what A is doing in the public foo() and the needs of B – if B needs A to do what it does in the public interface, then B should call the public interface and otherwise B should call the private interface. If that is true, then NVI creates a new source of bugs where sub class writers now have the option of calling the wrong interface. Is NVI normally done in some way that side-steps this issue?

  29. Yep, at least three times I had to stop myself from “fixing” that line to say

    auto pb = make_unique<derived>();

    which would actually have resolved a few problems (or at least changed them) but wouldn’t have preserved the semantics of the question. To preserve the semantics, I stuck with

     unique_ptr<base> pb = make_unique<derived>(); 

    However, I was actually _this_ close to applying the style I currently prefer of “auto var = Type{ expr };” for conversions, which in this case would be only very slightly more wordy

     auto pb = unique_ptr<base>{ make_unique<derived>() }; 

    I held back mainly because that style is still unusual to most people’s eyes and seeing the unique_ptr and the make_unique strung that close together might take some getting used to for readers… but now that I look at it, it does have the advantage of being very nice and explicit about what’s going on! Hmm. The more I look at it, the more I’m liking it…

  30. @KrzaQ: Yup, : should be :: — fixed, thanks.

    @mttpd: Yup, fixed. The original GotW had “void main()” and made a pedantic point about its conformance, and I decided it was a distraction and removed it by changing it to “int main()” in the question, but forgot to also update the solution.

  31. void main()

    — ouch! I don’t think this was really intended here! ;-)

  32. @Caleb, he’s explicit here as make_unique would return a unique_ptr to derived, not to base, which is what he wants here and which is the equivalent of the original code.

  33. I’m pretty sure that

    base:f;

    should be

    base::f;

    in “Well, remember that derived doesn’t declare using base:f; to bring the base functions named f into scope”. I’m sorry if this isn’t the way to report typos.

    While I’m at it, let me thank you for restarting GotW, I find your challenges stimulating.

  34. Apologies, that was meant to read that on the second line pb’s type will be

    unique_ptr<derived>
  35. Caleb, in your two lines the type of pb will be different. in the second line it will be a unique_ptr. If that’s what you want, fine, but it’s not the same as the first line, which includes an implicit conversion.

  36. Surely given your previous comments (and my own lazy inclinations), this line:

     unique_ptr<base> pb = make_unique<derived>();

    could become:

     auto pb = make_unique<derived>();

Comments are closed.