Cppcoreguidelines: C.128 Hiding Functions and Implicit Virtual/Override Functions Are Common and Useful

[BlackStarX42]

It is common to declare functions with the same name and different parameters in derived functions classes. A prominent example is the assignment operator, which typically has a parameter of the same type as the class where it is declared. It is not a mistake, but the pure intention. So reporting all and every hidden function yields a very high false positive rate.

On the other hand there really may be unintentionally hidden functions. The important cases are those where a function in the derived class could accidentally be called instead of the one in the derived base class. This is only possible in 2 cases:

• The base class function overloads are not introduced in the derived class definition with a using declaration ("hide-by-name") and all types of the parameter set (i.e. considering combinations of default parameters) of one function overload in the base class can implicitly be converted to the types of the parameter set of one function overload in the derived class,
• The base class function overloads are introduced in the derived class definition with a using declaration ("hide-by-signature") and all types of the parameters of one function overload of the base class match exactly the types of the parameters of one function overload of the derived class.

Furthermore, the guideline does not take into account that in some cases implicit overriding is useful and safe, for instance when the overridden function has no definition, which is frequently the case for interfaces (i.e. abstract classes with no non-static member function/variable definitions). Though the functions are technically overridden in the derived class, one would not consider this mechanism "overriding", but rather "implementing". Naturally adding override to all affected functions in the derived class does not make the intention any clearer. Also it would make turning a class into an interface implementation more bothersome because the developer would suddenly have to add override to all functions which now implement a function of the interface.

Additionally, the guideline does not consider situations where it may still be useful to mark destructors with virtual. If a derived class adds new virtual functions, the intention that the destructor needs to be virtual for this class should be made clear, independently of a possible virtual base class destructor. This also conforms to guideline C.35.

Moreover, the combination of virtual and final is actually useful because final does not necessarily imply overriding. This can be the case when a function shall be part of the polymorphic interface of a class, but shall already be final in the class where it is first declared. Accordingly, the combination of override and final is useful to state that a sealed function is indeed overriding another function and not a new virtual function.

I suggest to replace the current guideline with a more meaningful one:

• In case the overloads of functions with a certain name are not introduced in a derived class/struct with a using declaration ("hide-by-name"), there shall be no function overload in the derived class/struct which has a parameter set whose types can implicitly be converted from the types of a parameter set of a function overload in the base class/struct.
• In case the overloads of functions with a certain name are introduced in a derived class/struct with a using declaration ("hide-by-signature"), there shall be no function overload in the derived class/struct whose parameter types match exactly the parameter types of a function overload in the base class/struct.
• A function should be marked with override and should not be marked with virtual if it overrides a function which has a definition of a class which has a non-static member function with a definition (a class which is not an interface).
• A function should be marked with neither override nor virtual if it overrides a function which has no definition of a class which has no definition for any of the non-static member functions (an interface). This rule has lower priority than the rule above in the case that a class derives from both, an interface and a non-interface class which both happen to have a virtual function with the same name and signature.
• A destructor should be marked with virtual if and only if its containing class/struct adds new virtual functions, even if the base class/struct destructor is already virtual (in accordance with C.35). A destructor should never be marked with override.
• The usage of final is untouched by the guideline because sealing a function does not always imply overriding another function. final only implies a virtual function, which can be both, a new virtual function (which always has to be marked with virtual) or an overriding function, for which the rules above apply.

[ChrisGuzak]

in your first sentence you wrote in derived functions, I assume you mean in derived classes.

[cubbimew]

A prominent example is the assignment operator

operator= is indeed an example of name hiding that fails the enforcement of C.128 as currently worded ("Compare names in base and derived classes and flag uses of the same name that does not override" ).. I think the intent was to flag hiding of virtual member functions, as flagged by gcc's/c;lang's -Woverloaded-virtual.

For a more detailed justification of the guideline itself, see #1271

[brenoguim]

A function should be marked with neither override nor virtual if it overrides a function which has no definition.

Then you lose a nice compiler error when the owner of the interface decides to provide a base implementation on that interface and change the signature.

Though the functions are technically overridden in the derived class, one would not consider this mechanism "overriding", but rather "implementing".

Well, isn't "implementing" just an "override" of the absence of implementation into the presence of an implementation? :)

If you are writing a large software, you need all the static help you can get.

[BlackStarX42]

in your first sentence you wrote in derived functions, I assume you mean in derived classes.

Yes, thanks for pointing that out, fixed it.

A prominent example is the assignment operator

operator= is indeed an example of name hiding that fails the enforcement of C.128 as currently worded ("Compare names in base and derived classes and flag uses of the same name that does not override" ).. I think the intent was to flag hiding of virtual member functions, as flagged by gcc's/c;lang's -Woverloaded-virtual.
For a more detailed justification of the guideline itself, see #1271

Unintentional hiding of base class functions should always be flagged, regardless of whether the base class function is virtual or not. My goal was to clarify when actual unintentional hiding can happen, and when not.

A function should be marked with neither override nor virtual if it overrides a function which has no definition.

Then you lose a nice compiler error when the owner of the interface decides to provide a base implementation on that interface and change the signature.

You lose so many compiler errors just by using C++ ;-) Joke apart, The whole point of override is to make your intention explicit that you deliberately want to overrule the implementation of a base class. Basically you make the dependence between a derived class and a base class explicit. If the base class does not have any such implementation (i.e. an "interface"), nothing can be overruled. In this case the derived class's functionality is independent of the base class's functionality (because there is none). Everything the derived class could achieve by calling a non-static base class member function, it could as well achieve by calling the own non-static member function.

To add a non-static member function implementation to an existing interface suddenly changes the whole nature of that class, turning it into a general abstract class, which cannot be treated as an interface anymore. I realized now that I should change my proposal to not consider single function overrides regarding the usage of override but the complete set of functions of a base class, which decide if it is an interface or not.

Though the functions are technically overridden in the derived class, one would not consider this mechanism "overriding", but rather "implementing".

Well, isn't "implementing" just an "override" of the absence of implementation into the presence of an implementation? :)

No, that's exactly the difference as described above.

If you are writing a large software, you need all the static help you can get.

If you are writing a large software, you will be lucky to get as few false positives as possible.

[scraimer]

The whole point of override is to make your intention explicit that you deliberately want to overrule the implementation of a base class. Basically you make the dependence between a derived class and a base class explicit. If the base class does not have any such implementation (i.e. an "interface"), nothing can be overruled.

There's the matter of _when_ the code is written. Not all code is written at the same time.

I put this in bold, because when we discuss "the code" my mind automatically thinks of it as a monolithic item, when in reality it is a series of snapshots. Using override allows us to enforce integrity across snapshots, and not just within each one:

• When the derived class's function is written after the base class's function, the override keyword will give us a compiler error if the signatures don't match.
• When the base class's function is modified after the derived class is written, the override keyword will also give us a compiler error.

The second case is the usual "gotcha" when an API is changed. Without the override keyword, that case would cause name hiding, and run-time errors. And I always prefer compile-time errors to run-time errors.

[BlackStarX42]

It seems to me, we have different design goals for this guideline. Your goal is to design it in a way which makes you write code which subsequently triggers as much compiler output as possible (errors in your case). As a compiler needs to comply with the C++ language, it needs to accept everything the language allows, even doubtful cases, e.g. an unintentionally hidden function in a base class after you changed its signature (assuming it doesn't cause any other compilation error). This is why you have to map all the doubtful cases in advance to something that will definitely attract attention later (while compiling). You also need to do that in an undifferentiated way because you want to be steeled for everything that could happen in the future.

In contrast, my goal is to design the guideline in a way that it can be applied as a part of the compilation process itself. This way you have the chance to distinguish common and valid special cases, which is the essential difference to your approach.

I am sorry, it seems you got my basic idea wrong in your example, @scraimer. Of course, I do not advocate omitting override in regular situations where you override a base class function. I just pointed out one single special case when it is not meaningful to mark functions which (technically) override functions of the base class: If the base class is an interface. By definition, all non-static member functions in an interface are pure-virtual, which is why there must be functions in a derived class suitable to override all of them (otherwise you get a compilation error). This is why there cannot be any unintentionally hidden functions if you derive from an interface.

Also, any change to an interface will trigger a guideline violation (e.g. if you add a definition for a function, so the class cannot be treated as an interface anymore, you will suddenly get reports about missing overrides in the derived classes) or even a compiler error (e.g. if you changed the name or parameter types of one of the pure virtual functions).

[MikeGitb]

I still don't see what the harm is in just always requiring override. Why make the rule more complex by adding exceptions to it. What is the benefit?

[jwakely]

Naturally adding override to all affected functions in the derived class does not make the intention any clearer.

Every time somebody claims something is "natural" I get suspicious. The above statement doesn't seem at all uncontroversial.

Also it would make turning a class into an interface implementation more bothersome because the developer would suddenly have to add override to all functions which now implement a function of the interface.

Yes. But how often does that happen? And how bothersome is it really?

I see no justification for a change here.

[BlackStarX42]

I still don't see what the harm is in just always requiring override. Why make the rule more complex by adding exceptions to it. What is the benefit?

Also it would make turning a class into an interface implementation more bothersome because the developer would suddenly have to add override to all functions which now implement a function of the interface.

Yes. But how often does that happen? And how bothersome is it really?

The common way to design a type inheritance is to add a derived class after its base class, if you did it the other way round you would suddenly see your now-derived class faced with the new functionality provided by your new base class which would require redesign or at least intense review of your derived class. This is also the reason why it is reasonable to mark overriding functions with override in general as this would not work well if you frequently add base classes with virtual function to existing classes retroactively. I think, we all agree about that.

In contrast, an interface, by definition, does not contain any functionality (only pure virtual functions without definitions), which makes it easy to retroactively add it to existing classes which already support all the functions necessary to fulfil the contract of the interface. This way you just "enable" the interface on that class. It is not uncommon to do this if you _really_ apply OOP concepts in C++ (C++ is more than just template programming!). For instance, in .Net Framework recently many container types got IReadOnly variantes added retroactively. I know .Net is not C++, I just wanted to give you a clue about what I mean. (Though I think looking at concepts which were designed after the unpleasent experiences with languages like C++ can't be that wrong 😄.) So basically, the reason for my proposal is the pure nature of interfaces.

[MikeGitb]

you frequently add base classes with virtual function to existing classes retroactively. 

Never done that - ever. But if you do, you should really add those overrides to the derived classes to show that you are actually aware that those functions are now part of an polymorphic interface.

[BlackStarX42]

you frequently add base classes with virtual function to existing classes retroactively.

Never done that - ever. But if you do, you should really add those overrides to the derived classes to show that you are actually aware that those functions are now part of an polymorphic interface.

Neither have I ever done that. I never suggested to add base classes which contain functionality to existing classes retroactively but I also never suggested to omit override even if you do that.

The common way to design a type inheritance is to add a derived class after its base class, if you did it the other way round you would suddenly see your now-derived class faced with the new functionality provided by your new base class which would require redesign or at least intense review of your derived class. This is also the reason why it is reasonable to mark overriding functions with override in general as this would not work well if you frequently add base classes with virtual function to existing classes retroactively. I think, we all agree about that.

You can see two things when you look at the quote of my own paragraph:

• I actually already pointed out that you probably get problems if you add a base class with functionality to an existing class.
• The reason for the dependent clause you preferred to quote isolated was simply to emphasize that I actually agree that there are good and comprehensible reasons to mark overriding functions with override in general.

[MikeGitb]

I cited the wrong part of your post , my answer was related to:

In contrast, an interface, by definition, does not contain any functionality (only pure virtual functions without definitions), which makes it easy to retroactively add it to existing classes which already support all the functions necessary to fulfil the contract of the interface. 

Though my answer is the same to both of them. I've never done that and I think you should absolutely, positively add overrides to a function that became virtual retroactively. Generally speaking it should be visible in code, if a function is virtual or not and fundamentally I simply l don't see any benefit in distinguishing between overriding pure virtual functions and virtual functions with implementation for the purpose of this guideline.

If you are in favor of a different practice that is fine, as far as I can tell we have reached a point where we should agree to disagree. I'll wait and see what the editors have to say.

[BlackStarX42]

The practical relevance of retroactively adding interfaces is shown by my example about the IReadOnly interfaces in .Net. Also I have done it myself in some cases in C++.

The benefit from of not having to mark overriding functions in that case is that it is totally safe and reporting them anyway is almost certainly a waste of time because the developer has to add override without any purpose except for satisfying a rule. As an example, according to your reasoning, to find all uninitialized variables it would be sufficient to report all variables which are not assigned a value when they are declared. This is because always assigning a value when a variable is declared is a safe way to avoid uninitialized variables. So this rule is acceptable, is it? Really? No? Oh no, you missed some cases where it is actually totally safe to skip the report for a variable declaration, e.g. when its type is a class which contains a default constructor, or when the variable is assigned a value just the line after the declaration. I am certain you would not accept a rule/guideline or compiler warning which ignores these cases.

[jwakely]

the developer has to add override without _any_ purpose except for satisfying a rule.

Following the rule consistently means the rule can be kept simple, without (IMHO unnecessary and unhelpful) special cases as you're suggesting. It also helps protect the code from unintentional breakage later if refactored again.

Let's say you have:

struct Foo { int f(); };

And you change this to:

struct IFoo { virtual int f() = 0; };
struct Foo : IFoo { int f(); }

Later somebody changes it to:

struct IFoo { virtual int f() = 0; };
struct Foo : IFoo { int f(); }
struct Bar : Foo { int f(); };

And then later changes it again, but misses a change to the intermediate class:

struct IFoo { virtual int f(int = 0) = 0; };
struct Foo : IFoo { int f(); }
struct Bar : Foo { int f(int = 0); };

Congratulations, you have a bug. Had you used override right away the code wouldn't compile with that bug:

struct IFoo { virtual int f() = 0; };
struct Foo : IFoo { int f() override; }

If you don't like the rule, fine, don't follow it. But I don't think you're making a convincing argument that the guidelines should change, because your change wouldn't be an improvement.

[BlackStarX42]

I have already pointed out in my last answer why keeping rules too simple is a bad idea. I mean, I know some ways to make the current guideline wording even simpler but I am pretty sure you won't like them.

Also:

struct IFoo { virtual int f() = 0; };
struct Foo : IFoo { int f(); }
struct Bar : Foo { int f(); };

In this case you would already have a guideline violation because you would have to mark Bar::f() with override (because its base class Foo is not an interface).

struct IFoo { virtual int f(int = 0) = 0; };
struct Foo : IFoo { int f(); }
struct Bar : Foo { int f(int = 0); };

This would be another guideline violation as Foo::f() would be reported as unintentionally hiding IFoo::f(int = 0) (because one possible parameter set of IFoo::f can be implicitly converted to a suitable parameter set of Foo::f). Same goes for Bar::f(int = 0) with Foo::f().

[phillipjohnston]

The benefit from of not having to mark overriding functions in that case is that it is totally safe and reporting them anyway is almost certainly a waste of time because the developer has to add override without any purpose except for satisfying a rule.

This is a specious argument. There are other benefits to using the override keyword than blindly satisfying a rule. I willingly mark functions as "override" without even having a guidelines checker to yell at me! If I have a class which mixes overridden functions with functions not derived in the base, it can be quite difficult to discern at a glance where the definition comes from. If I see "override", I immediately know this comes from a base class. This is worth the "waste of time" (120wpm -> 1/2 sec to write the word, or 2 chars + enter for autocomplete).

[jwakely]

@BlackStarX42 add override to Bar::f() and nothing changes. Change the signatures from int() to int(int) and it doesn't have a compatible parameter list, but the bug remains.

At some point you're going to have to accept that nobody likes your proposed change.

[BlackStarX42]

@phillipjohnston
That is the whole point. An interface has no meaningful non-static functionality because it has no non-static function definitions. How can an overriding function "come" from that interface? Also, what if you are overriding the function of 2 base classes with one function, how do you denote that your overriding function "comes" from this or that base class. I think there is no such concept like "a function comes from a base class".

@jwakely
In which case do you want me to add override or change int() to int(int)?
Please do not tell me what I have to accept. I will accept the denial of the proposed change if and when I see a valid argument against it. So far, the main impression that I get is that some people only argue out of a basic refusing stance.

[phillipjohnston]

@BlackStarX42

An overriding function "comes" from an abstract base class because you derived from it. The function prototype didn't originate in the derived class. So where did the requirement to implement it come from, if not the base?

Also, what if you are overriding the function of 2 base classes with one function, how do you denote that your overriding function "comes" from this or that base class.

If you are deriving from two non-orthogonal base classes, you're going to run into bigger problems than override's semantic documentation value.

[BlackStarX42]

@phillipjohnston Yes, the function prototype comes from the base class. This is an essential difference to a function definition coming from a base class. When I see an override I would expect some functionality (which assumes a definition) to be overruled as I don't see a use to mark non-existing functionalities to be overruled.

I think you are right that overriding 2 functions with 1 function in the derived class is causing many more troubles.

[jwakely]

When I see an override I would expect some functionality (which assumes a definition) to be overruled as I don't see a use to mark non-existing functionalities to be overruled.

Well there's your problem. That's not what override means, and not how everybody else thinks of it.

[BlackStarX42]

So are we talking about what override means or about what everybody thinks of it?

Regarding what override means, well, it means what the language specification says, it specifies that a virtual function overrides another virtual function. The language specification does not say that override is mandatory. It seems you do interpret it as mandatory anyway, but the language specification says something similar about virtual as about override but you don't seem to interpret virtual as mandatory. Why is that? It is because you saw a reason for it. And all I do is proposing a reason to not interpret overrideas mandatory neither. I mean, otherwise we could just make virtual mandatory too as it would simplify the guideline even more.

[jwakely]

but you don't seem to interpret virtual as mandatory

There's a guideline saying exactly one of virtual, override or final should be used on virtual functions. The virtual is redundant if one of the others is present. That has been discussed to death in another issue ticket here, so you can find the justification there.

At some point you're going to have to accept that nobody likes your proposed change.

[BlackStarX42]

but you don't seem to interpret virtual as mandatory

There's a guideline saying exactly one of virtual, override or final should be used on virtual functions.

Well, yep, I know that guideline, maybe because we are in the middle of a discussion about that very guideline.

The virtual is redundant if one of the others is present. That has been discussed to death in another issue ticket here, so you can find the justification there.

I know that. But again you missed the point of my paragraph. I wanted to emphasize that virtual is obvsiouly not mandatory, so why is override. Why isnt't it conceivable that there are even cases when override is not mandatory? Because you desparately want to have exactly one of the keywords in the declaration of a virtual function? I have to disappoint you, the combination of virtual and final cannot be expressed by only one of them.

At some point you're going to have to accept that nobody likes your proposed change.

You are repeating yourself. Can I conclude that you don't have any valid arguments anymore about my proposal?

[jwakely]

[class.mem] p14 is the answer to your original question, before the ninja edit.

Can I conclude that you don't have any valid arguments anymore about my proposal?

Sure. Can I conclude you have none in favour of your proposal either?

[BlackStarX42]

[class.mem] p14 is the answer to your original question, before the ninja edit.

The original question, which I prefered to removed, was if you can show me the spot in the language specification that tells that final implies virtual? In draft n4800, I found that you shall not repeat any of override or final in the same function delcaration and that they shall appear only in the first declaration of a function. Other versions of the draft only mention either the naming of members or the requirement of complete types for data members. Did I miss something?

Sure.

Of course, because you don't need any argument to just leave everything the way it was, I admit that's very convenient.

Can I conclude you have none in favour of your proposal either?

Of course not, for exactly the same reason as above.

[hsutter]

Folks, let's please keep it calm and technical. :)

The most important thing we can do to help programmers make their code both simpler and more robust is to enable them to declare their intent, and that is why override has been so successful. I strongly agree that C.128 is correct as written: You should declare virtual functions using exactly one of virtual, override, or final. This has been analyzed in depth and is a correct rule, not an oversimplified rule; there have been no valid cases presented where following the rule is incorrect, misleading, or difficult, and it is known in C++ and other languages to frequently diagnose bugs with zero false positives which is about as Grail-like as you can get for a rule. Please see also:

• #1271
• #713
• #423

However, I do agree that we should improve the Enforcements to apply to only virtual functions in the first and third bullets:

• Compare __virtual function__ names in base and derived classes and flag uses of the same name that does not override.
• Flag overrides with neither override nor final.
• Flag __virtual__ function declarations that use more than one of virtual, override, and final.

[BlackStarX42]

I see your suggestion as a compromise to exclude non-virtual functions from the guideline for now. I am ok with that because it would remove the need to flag the most common false positives, the assignment operators.

I would like to suggest to extend the first enforcement to only flag a function if its overloads were not introduced in the derived class with a using declaration because only then you can hide virtual functions. If you introduce the overloads of a function in a derived class and add a declaration for a function with the same name, you end up either overriding an existing function of the base class (which needs to be marked according to bullet 2), or adding an overload with a different signature than the existing overloads, which is ok, or you hiding a non-virtual function with exactly the same signature of the base class but this is not scope of this guideline anymore according to Mr Sutter's proposal. See https://rextester.com/SWUGO49984. So I suggest to add the following the the 1st enforcement:

• Compare virtual function names in base and derived classes and flag uses of the same name that does not override if the overloads of the function were not introduced in the derived class (i.e. via a using declaration)

I would also like to point out that the guideline mentions the marking of destructors but lacks an according enforcement for it. In my opinion the current wording about this aspect is unfavorable. If you add new virtual functions to a class, it should be mandatory to mark the destructor virtual, independently of whether the base class already did the same. If the base class definition gets changed in a way that its destructor is not virtual anymore, the destructor of the derived class still needs to be, so I suggest to add this enforcement (and change the guideline description accordingly):

• Flag classes which contain a declaration for a new virtual function (i.e. one that is not overriding another virtual function) and whose destructor is not marked virtual.

[hsutter]

Editors call: We think the destructor case is covered by "Make base class destructors public and virtual, or protected and nonvirtual." That guidance applies the same in base and derived classes, or when adding a virtual function or not.