Requiring virtual function overrides to use override keyword

23,515

Solution 1

Looks like the GCC 5.1 release added exactly the warning I was looking for:

-Wsuggest-override
       Warn about overriding virtual functions that are not marked with the override keyword.

Compiling with -Wsuggest-override -Werror=suggest-override would then enforce that all overrides use override.

Solution 2

There are two things you can do.

First, Clang 3.5 and higher have a -Winconsistent-missing-override warning (triggered by -Wall). This does not quite work for your example, but only if you would add a void foo() override {} to class B and not in class C. What you actually want is -Wmissing-override, to locate all missing override, not just the inconsistently missing ones. That is currently not provided, but you might complain on the Clang mailinglist and they might add it.

Second, you use Howard Hinnant's trick to temporarily add final to the base class member function and recompile. The compiler will then locate all further derived classes that attempt to override the virtual base member function. You can then fix the missing ones. It's a bit more work and requires frequent rechecking when your class hierarchy expands.

Solution 3

The problem I see with -Werror=suggest-override is that it does not allow you to write the following:

void f() final {...}

Even though there is an implicit override here. The -Werror=suggest-override does not ignore this (like it should, since the override is redundant in this case)

But it is more complicated than that... If you write

virtual void f() final {...}

It means a completely different thing than

virtual void f() override final {...}

The first case does not need to override anything! The second does.

So I am assuming the GCC check is implemented this way (i.e. to sometimes accept the redundant override) in order to get the last case right. But this does not play well e.g. with clang-tidy, which will correctly remove the override when final is sufficient (but then GCC compilation will fail...)

Share:
23,515

Related videos on Youtube

Barry
Author by

Barry

I like c++. I am also somewhat obsessed with SO. Users I subscribe to, in no particular order: Jonathan Wakely, T.C., Columbo, Yakk, dyp Various badge milestones: 1st to receive the gold C++20 badge 1st to receive the gold C++17 badge 2nd to receive the gold language-lawyer badge 2nd to receive the gold C++14 badge 4th to receive the silver C++14 badge 8th to receive the gold templates badge 11th to receive the bronze C++14 badge 29th to receive the gold C++11 badge 31st to receive the silver templates badge 256th to receive the gold C++ badge, or 0x0100th if you prefer

Updated on July 09, 2022

Comments

  • Barry
    Barry almost 2 years

    C++11 added override to ensure that member functions you write that you intend to override base-class virtual functions actually do (or won't compile).

    But in a large object hierarchy, sometimes you could accidentally end up writing a member function that overrides a base-class virtual when you didn't intend it! For instance:

    struct A {
        virtual void foo() { }  // because obviously every class has foo().
    };
    
    struct B : A { ... };
    
    class C : B {
    private:
        void foo() {
            // was intended to be a private function local to C
            // not intended to override A::foo(), but now does
        }
    };
    

    Is there some compiler flag/extension that would at least issue a warning on C::foo ? For readability and correctness, I just would like to enforce that all overrides use override.

    • MSalters
      MSalters over 9 years
      In reality, your function will be named GazimpleWidget(Widget& w) and obviously C::GazimpleWidget(Widget& w) still gazimples widgets. You only get such problems when you try to abbreviate C::GazimpleWidget( ) to C::GW( ). Don't do that.
    • Anton Savin
      Anton Savin over 9 years
    • Barry
      Barry over 9 years
      @MSalters I do not understand the comment.
    • MSalters
      MSalters over 9 years
      @Barry: The problem with your simplified example is that foo is misleading. Real functions with real names are far less likely to collide unintentionally. If they are named the same, they should be doing the same and then overriding is probably not an error.
    • Barry
      Barry over 9 years
      @MSalters "less likely...", "should be...", "probably not...". Yes, obviously. The entire point of the question is that the unlikely case happened, the function was incorrectly implemented, and I would like to verify this at compile-time.
  • Marco Freudenberger
    Marco Freudenberger over 6 years
    The question was about how to enable a compiler warning when an the "override" keyword was not used on an overridden method, not how to enbale warnings on missusage of the override keyword.
  • Chris Uzdavinis
    Chris Uzdavinis almost 6 years
    I agree with you. However, if we follow the advice to only use the "virtual" keyword on functions when first introducing a new virtual function, the issue you show where a final function may-or-may-not-override goes away. (This disallows code that introduces a virtual function and implements it as final, which seems nonsensical anyway.) Then the meaning is clear: "virtual" = new non-overriding virtual function, while "override" or "final" both indicate they are overrides (and do NOT have "virtual") and both should satisfy the g++ warning (but unfortunately doesn't.)
  • underscore_d
    underscore_d about 5 years
    Declaring and finalising a base virtual function in the same breath, thus defeating the point of it being virtual, seems totally pointless. I'd almost say the language should disallow this, then compilers wouldn't have to ponder the philosophy of it.
  • Robert Skinner
    Robert Skinner over 4 years
    I agree with @underscore_d and Scott Meyers on this one. If there is only one way to implement f(), take a firm stand and declare it non-virtual.
  • VainMan
    VainMan over 2 years
    They removed this option from their docs for GCC 10.3 and later versions. The option still works on those versions though.
  • Barry
    Barry over 2 years
    @VainMan Half the warnings are in Dialect Options for some reason, it's very confusing.