CXX11 patch to warn if 'override' is missing on overriding virtual function

Douglas Gregor dgregor at apple.com
Tue Sep 30 16:11:07 PDT 2014


> On Sep 30, 2014, at 2:58 PM, Reid Kleckner <rnk at google.com> wrote:
> 
> +cc clang-tidy people, since they rolled this kind of stuff out already.
> 
> Index: test/Parser/cxx0x-in-cxx98.cpp
> ===================================================================
> --- test/Parser/cxx0x-in-cxx98.cpp	(revision 218519)
> +++ test/Parser/cxx0x-in-cxx98.cpp	(working copy)
> @@ -10,11 +10,12 @@
>  
>  struct B {
>    virtual void f();
> -  virtual void g();
> +  virtual void g(); // expected-note {{overridden virtual function is here}}
>  };
>  struct D final : B { // expected-warning {{'final' keyword is a C++11 extension}}
>    virtual void f() override; // expected-warning {{'override' keyword is a C++11 extension}}
> -  virtual void g() final; // expected-warning {{'final' keyword is a C++11 extension}}
> +  virtual void g() final; // expected-warning {{'final' keyword is a C++11 extension}} \
> +			  // expected-warning {{'g' overrides a member function but is not marked 'override'}}
>  };
>  
>  void NewBracedInitList() {
> 
> Should we really be suggesting that users add C++11 features like override when -Wcxx98-compat is on? That seems undesirable.

To get this warning, the user will already have written ‘override’, demonstrating that they don’t care about -Wcxx98-compat warnings at all. I suspect there isn’t a real use case here.

> 
> Index: test/Parser/cxx0x-decl.cpp
> ===================================================================
> --- test/Parser/cxx0x-decl.cpp	(revision 218519)
> +++ test/Parser/cxx0x-decl.cpp	(working copy)
> @@ -83,13 +83,13 @@
>  
>  namespace FinalOverride {
>    struct Base {
> -    virtual void *f();
> +    virtual void *f(); // expected-note {{overridden virtual function is here}}
>      virtual void *g();
>      virtual void *h();
>      virtual void *i();
>    };
>    struct Derived : Base {
> -    virtual auto f() -> void *final;
> +    virtual auto f() -> void *final; // expected-warning {{'f' overrides a member function but is not marked 'override'}}
>      virtual auto g() -> void *override;
>      virtual auto h() -> void *final override;
>      virtual auto i() -> void *override final;
> 
> Why make this suggestion? 'override' is redundant in the presence of 'final’,

That’s not true. “override” says that you’re overriding something from the base class, potentially changing the behavior from what your base class would have provided. “final” says that your own derived classes can’t change the behavior further. Those are separate concerns.

> assuming that we warn on final virtual methods that don't override anything, which we probably should.

Sure, that warning would make sense.

	- Doug






More information about the cfe-commits mailing list