https://github.com/fwinkl created
https://github.com/llvm/llvm-project/pull/144916
Add a new `AllowVirtual` option to 'modernize-use-override', which does not
remove the `virtual` specifier. This is useful because some coding guidelines
may suggest to use `virtual ... override` for clarity. See the new unit test
for examples.
>From eb3aedaf2159ea3d69cdcb9cb15ae8e86a63a008 Mon Sep 17 00:00:00 2001
From: Frank Winklmeier <frank.winklme...@cern.ch>
Date: Thu, 19 Jun 2025 17:23:46 +0200
Subject: [PATCH] [clang-tidy] Add option to keep virtual in
'modernize-use-override'
Add a new `AllowVirtual` option to 'modernize-use-override', which does
not remove the `virtual` specifier. This is useful because some coding
guidelines may suggest to use `virtual ... override` for clarity.
---
.../clang-tidy/modernize/UseOverrideCheck.cpp | 13 +++++++---
.../clang-tidy/modernize/UseOverrideCheck.h | 1 +
.../checks/modernize/use-override.rst | 8 +++++-
.../modernize/use-override-allow-virtual.cpp | 25 +++++++++++++++++++
4 files changed, 43 insertions(+), 4 deletions(-)
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/modernize/use-override-allow-virtual.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
index fd5bd9f0b181b..c18bd68a99b37 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
@@ -22,6 +22,7 @@ UseOverrideCheck::UseOverrideCheck(StringRef Name,
ClangTidyContext *Context)
IgnoreTemplateInstantiations(
Options.get("IgnoreTemplateInstantiations", false)),
AllowOverrideAndFinal(Options.get("AllowOverrideAndFinal", false)),
+ AllowVirtual(Options.get("AllowVirtual", false)),
OverrideSpelling(Options.get("OverrideSpelling", "override")),
FinalSpelling(Options.get("FinalSpelling", "final")) {}
@@ -30,6 +31,7 @@ void
UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "IgnoreTemplateInstantiations",
IgnoreTemplateInstantiations);
Options.store(Opts, "AllowOverrideAndFinal", AllowOverrideAndFinal);
+ Options.store(Opts, "AllowVirtual", AllowVirtual);
Options.store(Opts, "OverrideSpelling", OverrideSpelling);
Options.store(Opts, "FinalSpelling", FinalSpelling);
}
@@ -111,15 +113,19 @@ void UseOverrideCheck::check(const
MatchFinder::MatchResult &Result) {
unsigned KeywordCount = HasVirtual + HasOverride + HasFinal;
if ((!OnlyVirtualSpecified && KeywordCount == 1) ||
+ (HasVirtual && HasOverride && AllowVirtual) ||
(!HasVirtual && HasOverride && HasFinal && AllowOverrideAndFinal))
return; // Nothing to do.
std::string Message;
if (OnlyVirtualSpecified) {
- Message = "prefer using '%0' or (rarely) '%1' instead of 'virtual'";
+ if (AllowVirtual)
+ Message = "add '%0' or (rarely) '%1' for 'virtual' function";
+ else
+ Message = "prefer using '%0' or (rarely) '%1' instead of 'virtual'";
} else if (KeywordCount == 0) {
Message = "annotate this function with '%0' or (rarely) '%1'";
- } else {
+ } else if (!AllowVirtual) {
StringRef Redundant =
HasVirtual ? (HasOverride && HasFinal && !AllowOverrideAndFinal
? "'virtual' and '%0' are"
@@ -226,7 +232,8 @@ void UseOverrideCheck::check(const MatchFinder::MatchResult
&Result) {
CharSourceRange::getTokenRange(OverrideLoc, OverrideLoc));
}
- if (HasVirtual) {
+ // Remove virtual unless requested otherwise
+ if (HasVirtual && !AllowVirtual) {
for (Token Tok : Tokens) {
if (Tok.is(tok::kw_virtual)) {
std::optional<Token> NextToken =
diff --git a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
index 2c624f48fcc85..6879221ab4284 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
@@ -29,6 +29,7 @@ class UseOverrideCheck : public ClangTidyCheck {
const bool IgnoreDestructors;
const bool IgnoreTemplateInstantiations;
const bool AllowOverrideAndFinal;
+ const bool AllowVirtual;
const StringRef OverrideSpelling;
const StringRef FinalSpelling;
};
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-override.rst
b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-override.rst
index f8f34794af749..d3ed73ef35a51 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-override.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-override.rst
@@ -4,7 +4,8 @@ modernize-use-override
======================
Adds ``override`` (introduced in C++11) to overridden virtual functions and
-removes ``virtual`` from those functions as it is not required.
+removes ``virtual`` from those functions (unless the `AllowVirtual` option is
+used).
``virtual`` on non base class implementations was used to help indicate to the
user that a function was virtual. C++ compilers did not use the presence of
@@ -40,6 +41,11 @@ Options
members, such as ``gcc -Wsuggest-override``/``gcc
-Werror=suggest-override``.
Default is `false`.
+.. option:: AllowVirtual
+
+ If set to `true`, ``virtual`` will not be removed by this check when adding
+ ``override`` or ``final``. Default is `false`.
+
.. option:: OverrideSpelling
Specifies a macro to use instead of ``override``. This is useful when
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-override-allow-virtual.cpp
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-override-allow-virtual.cpp
new file mode 100644
index 0000000000000..17be170e0d9cd
--- /dev/null
+++
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-override-allow-virtual.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- \
+// RUN: -config="{CheckOptions: {modernize-use-override.AllowVirtual: true}}"
+
+struct Base {
+ virtual ~Base();
+ virtual void a();
+ virtual void b();
+ virtual void c();
+};
+
+struct Derived : public Base {
+ virtual ~Derived() override;
+
+ virtual void a() override;
+ // CHECK-MESSAGES-NOT: warning:
+ // CHECK-FIXES: {{^}} virtual void a() override;
+
+ virtual void b();
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: add 'override'
+ // CHECK-FIXES: {{^}} virtual void b() override;
+
+ void c();
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate
+ // CHECK-FIXES: {{^}} void c() override;
+};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
