PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, carlosgalvezp.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fixed false positive in modernize-use-override check that occurred
when warnings were generated for template classes that were valid
for some instantiations but potentially invalid for others.
Template instantiations are now excluded from the check.

Fixes #38276.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147924

Files:
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-override.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-override.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-override.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-override.cpp
@@ -274,16 +274,16 @@
 
 template <typename T> struct DerivedFromTemplate : public TemplateBase<T> {
   virtual void f(T t);
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
-  // CHECK-FIXES: {{^}}  void f(T t) override;
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES-NOT: {{^}}  void f(T t) override;
 };
 void f() { DerivedFromTemplate<int>().f(2); }
 
 template <class C>
 struct UnusedMemberInstantiation : public C {
   virtual ~UnusedMemberInstantiation() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using
-  // CHECK-FIXES: {{^}}  ~UnusedMemberInstantiation() override {}
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:11: warning: prefer using
+  // CHECK-FIXES-NOT: {{^}}  ~UnusedMemberInstantiation() override {}
 };
 struct IntantiateWithoutUse : public UnusedMemberInstantiation<Base> {};
 
@@ -316,3 +316,56 @@
   // CHECK-FIXES: {{^}}  void d() override try
   { e(); } catch(...) { f(); }
 };
+
+namespace PR38276 {
+  struct Base {
+    virtual void foo();
+  };
+
+  struct Base2 {
+    virtual void foo2();
+  };
+
+  template<typename T>
+  struct Derived : T {
+    virtual void foo();
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:18: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions,modernize-use-override]
+    // CHECK-FIXES-NOT: {{^}}    void foo() override;{{$}}
+    virtual void foo2();
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:18: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions,modernize-use-override]
+    // CHECK-FIXES-NOT: {{^}}    void foo2() override;{{$}}
+  };
+
+  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 {
+    virtual void boo();
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:18: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions,modernize-use-override]
+    // CHECK-FIXES-NOT: {{^}}    void boo() override;{{$}}
+    virtual void boo2();
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:18: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions,modernize-use-override]
+    // CHECK-FIXES-NOT: {{^}}    void boo2() override;{{$}}
+    virtual void boo3();
+    // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions,modernize-use-override]
+    // CHECK-FIXES: {{^}}    void boo3() override;{{$}}
+  };
+
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -235,6 +235,11 @@
   functions containing macros or preprocessor directives, and out-of-line special
   member functions in unions.
 
+- Fixed false positive in :doc:`modernize-use-override
+  <clang-tidy/checks/modernize/use-override>` check that occurred when warnings
+  were generated for template classes that were valid for some instantiations
+  but potentially invalid for others.
+
 - Fixed reading `HungarianNotation.CString.*` options in
   :doc:`readability-identifier-naming
   <clang-tidy/checks/readability/identifier-naming>` check.
Index: clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
@@ -30,12 +30,16 @@
 }
 
 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();
+  Finder->addMatcher(
+      cxxMethodDecl(isOverride(),
+                    unless(ast_matchers::isTemplateInstantiation()),
+                    IgnoreDestructorMatcher)
+          .bind("method"),
+      this);
 }
 
 // Re-lex the tokens to get precise locations to insert 'override' and remove
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to