[Warnings] final class with virtual functions

Hi,
I was wondering if we should warn about cases like this:

struct A final {

virtual int foo () {return 42;}

// virtual doesn’t make any sense because it is final base class

};

or

struct A {

virtual int foo () final {return 42;}

// the same thing

};

So in both cases it sounds like a bug, specially in the first case.

I don’t see any resonable cases where this code would make sense (what you could not achieve with override keyword).

If emiting warning in these cases make sense, dibs on that feature.

Piotr

Hi,
I was wondering if we should warn about cases like this:

struct A final {

   virtual int foo () {return 42;}

   // virtual doesn't make any sense because it is final base class

};

or

struct A {

  virtual int foo () final {return 42;}

  // the same thing

};

So in both cases it sounds like a bug, specially in the first case.

I don't think it sounds like a bug (at least, nothing we'd want to
diagnose in the frontend).

Prior to the override keyword, some coding style guidelines
recommended you write virtual on all virtual functions (instead of
just the first declaration of such a function in a base class) to make
it clear that the function is one that can be overridden. I can easily
see code being written that way, but later getting a final specifier
slapped on the class without changing all of the methods to use the
override keyword. The behavior of the class is the same either way,
which is why this doesn't seem like a bug to me.

Suggesting that the user make use of the override keyword instead
makes sense to me, but that doesn't mean the code is buggy. I think
such a check in the frontend would be needlessly chatty. However, I
think this would make sense as a clang-tidy check for the readability
or modernize module. We have modernize-use-override already, so
perhaps we could either extend that check or create a new check.

~Aaron

I think it’s a bug, but I also think it’s unlikely to happen.

The warning is about declaring a virtual method that cannot be overridden.
It wouldn’t trigger for methods that are overriding, whether the override keyword is used it not.

Ah, I missed the fact that there was no base class in the examples and
was thinking of this situation:

struct B {
  virtual void f() { /* ... */ }
};

struct B final : D {
  virtual void f() { /* ... */ }
};

So long as we don't warn on this, I'm fine with it being a frontend diagnostic.

~Aaron

-others because I'm unsure :slight_smile:

Arent there some forms of RTTI that are not available unless you have a
virtual function? I swear i ran into a case where I added virtual functions
to a class solely because I needed it to behave well in a place where RTTI
was used.

Jared

+ Rest

That's a valid concern, I am not really sure. But does it make sense to use
RTTI on "static" classes like this?
I never worked on code base that would use RTTI, it was always some hand
crafted solutions, so I am not really familiar with this problem.

Piotr