2ff370f452
There's never any point to adding a `virtual` specifier to methods in a `final` class, since the class can't be subclassed. This adds a warning when we notice this happening, as suggested in #131108. We don't currently implement the second part of the suggestion, to warn on `virtual` methods which are never overridden anywhere. Although it's feasible to do this for things with internal linkage (so we can check at the end of the TU), it's more complicated to implement and it's not clear it's worth the effort. I tested the warning by compiling chromium and clang itself. Chromium resulted in [277 warnings across 109 files](https://github.com/user-attachments/files/19234889/warnings-chromium.txt), while clang had [38 warnings across 29 files](https://github.com/user-attachments/files/19234888/warnings-clang.txt). I inspected a subset of the warning sites manually, and they all seemed legitimate. This warning is very easy to fix (just remove the `virtual` specifier) and I haven't seen any false positives, so it's suitable for on-by-default. However, I've currently made it off-by-default because it fires at several places in the repo. I plan to submit a followup PR fixing those places and enabling the warning by default.
32 lines
1.3 KiB
C++
32 lines
1.3 KiB
C++
// RUN: %clang_cc1 -fsyntax-only -verify -Wunnecessary-virtual-specifier -Wno-inconsistent-missing-override %s
|
|
|
|
struct Foo final {
|
|
Foo() = default;
|
|
virtual ~Foo() = default; // expected-warning {{virtual method}}
|
|
virtual Foo& operator=(Foo& other) = default; // expected-warning {{virtual method}}
|
|
virtual Foo& operator=(Foo&& other) = default; // expected-warning {{virtual method}}
|
|
void f();
|
|
virtual void f(int); // expected-warning {{virtual method}}
|
|
int g(int x) { return x; };
|
|
virtual int g(bool); // expected-warning {{virtual method}}
|
|
static int s();
|
|
};
|
|
|
|
struct BarBase {
|
|
virtual ~BarBase() = delete;
|
|
virtual void virt() {}
|
|
virtual int virt2(int);
|
|
virtual bool virt3(bool);
|
|
int nonvirt();
|
|
};
|
|
|
|
struct Bar final : BarBase {
|
|
~Bar() override = delete;
|
|
void virt() override {};
|
|
virtual int virt2(int) override; // `virtual ... override;` is a common pattern, so don't warn
|
|
virtual bool virt3(bool); // Already virtual in the base class; triggers
|
|
// -Winconsistent-missing-override or -Wsuggest-override instead
|
|
virtual int new_virt(bool); // expected-warning {{virtual method}}
|
|
int nonvirt();
|
|
};
|