[clang-tools-extra] 9556409 - [clang-tidy] Exclude template instantiations in modernize-use-override

[Piotr Zegar via cfe-commits]

Author: Piotr Zegar
Date: 2023-04-15T10:16:03Z
New Revision: 9556409957bbf85ca751ccff8f3a4d981a094b73

URL: https://github.com/llvm/llvm-project/commit/9556409957bbf85ca751ccff8f3a4d981a094b73
DIFF: https://github.com/llvm/llvm-project/commit/9556409957bbf85ca751ccff8f3a4d981a094b73.diff

LOG: [clang-tidy] Exclude template instantiations in modernize-use-override

Added IgnoreTemplateInstantiations option to modernize-use-override
check. Allows to ignore virtual function overrides that are part of
template instantiations. This can be useful in cases where the use
of the "override" keyword is not appropriate or causes issues with
template specialization.

Fixes #38276.

Reviewed By: carlosgalvezp

Differential Revision: https://reviews.llvm.org/D147924

Added: 
    clang-tools-extra/test/clang-tidy/checkers/modernize/use-override-templates.cpp

Modified: 
    clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
    clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/modernize/use-override.rst

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
index a3201c825d387..824d037e4552c 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
@@ -18,24 +18,35 @@ namespace clang::tidy::modernize {
 UseOverrideCheck::UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       IgnoreDestructors(Options.get("IgnoreDestructors", false)),
+      IgnoreTemplateInstantiations(
+          Options.get("IgnoreTemplateInstantiations", false)),
       AllowOverrideAndFinal(Options.get("AllowOverrideAndFinal", false)),
       OverrideSpelling(Options.get("OverrideSpelling", "override")),
       FinalSpelling(Options.get("FinalSpelling", "final")) {}
 
 void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IgnoreDestructors", IgnoreDestructors);
+  Options.store(Opts, "IgnoreTemplateInstantiations",
+                IgnoreTemplateInstantiations);
   Options.store(Opts, "AllowOverrideAndFinal", AllowOverrideAndFinal);
   Options.store(Opts, "OverrideSpelling", OverrideSpelling);
   Options.store(Opts, "FinalSpelling", FinalSpelling);
 }
 
 void UseOverrideCheck::registerMatchers(MatchFinder *Finder) {
-  if (IgnoreDestructors)
-    Finder->addMatcher(
-        cxxMethodDecl(isOverride(), unless(cxxDestructorDecl())).bind("method"),
-        this);
-  else
-    Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
+
+  auto IgnoreDestructorMatcher =
+      IgnoreDestructors ? cxxMethodDecl(unless(cxxDestructorDecl()))
+                        : cxxMethodDecl();
+  auto IgnoreTemplateInstantiationsMatcher =
+      IgnoreTemplateInstantiations
+          ? cxxMethodDecl(unless(ast_matchers::isTemplateInstantiation()))
+          : cxxMethodDecl();
+  Finder->addMatcher(cxxMethodDecl(isOverride(),
+                                   IgnoreTemplateInstantiationsMatcher,
+                                   IgnoreDestructorMatcher)
+                         .bind("method"),
+                     this);
 }
 
 // Re-lex the tokens to get precise locations to insert 'override' and remove

diff  --git a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
index 332e65b198c97..2c624f48fcc85 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
@@ -27,6 +27,7 @@ class UseOverrideCheck : public ClangTidyCheck {
 
 private:
   const bool IgnoreDestructors;
+  const bool IgnoreTemplateInstantiations;
   const bool AllowOverrideAndFinal;
   const StringRef OverrideSpelling;
   const StringRef FinalSpelling;

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 37024576ee99c..7536aba58af50 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -246,6 +246,11 @@ Changes in existing checks
   functions containing macros or preprocessor directives, and out-of-line special
   member functions in unions.
 
+- Improved :doc:`modernize-use-override
+  <clang-tidy/checks/modernize/use-override>` check with new
+  `IgnoreTemplateInstantiations` option to optionally ignore virtual function
+  overrides that are part of template instantiations.
+
 - Fixed reading `HungarianNotation.CString.*` options in
   :doc:`readability-identifier-naming
   <clang-tidy/checks/readability/identifier-naming>` check.

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 61ffc955d92eb..0440ab855ea7b 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
@@ -27,6 +27,11 @@ Options
 
    If set to `true`, this check will not diagnose destructors. Default is `false`.
 
+.. option:: IgnoreTemplateInstantiations
+
+   If set to `true`, instructs this check to ignore virtual function overrides
+   that are part of template instantiations. Default is `false`.
+
 .. option:: AllowOverrideAndFinal
 
    If set to `true`, this check will not diagnose ``override`` as redundant

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-override-templates.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-override-templates.cpp
new file mode 100644
index 0000000000000..12f8cf447618b
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-override-templates.cpp
@@ -0,0 +1,47 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-override.IgnoreTemplateInstantiations, value: true}]}"
+
+struct Base {
+  virtual void foo();
+};
+
+struct Base2 {
+  virtual void foo2();
+};
+
+template<typename T>
+struct Derived : T {
+  // should not warn, comes from template instance
+  virtual void foo();
+  virtual void foo2();
+};
+
+void test() {
+  Derived<Base> b;
+  Derived<Base2> b2;
+}
+
+template<typename T>
+struct BaseS {
+  virtual void boo();
+};
+
+template<>
+struct BaseS<int> {
+  virtual void boo2();
+};
+
+struct BaseU {
+  virtual void boo3();
+};
+
+template<typename T>
+struct Derived2 : BaseS<T>, BaseU {
+  // should not warn, comes from template instance
+  virtual void boo();
+  virtual void boo2();
+  // should warn, comes from non-template BaseU
+  virtual void boo3();
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^  }}void boo3() override;
+};