[clang-tools-extra] r230765 - [clang-tidy] Various improvements in misc-use-override

Alexander Kornienko alexfh at google.com
Fri Feb 27 08:50:33 PST 2015


Author: alexfh
Date: Fri Feb 27 10:50:32 2015
New Revision: 230765

URL: http://llvm.org/viewvc/llvm-project?rev=230765&view=rev
Log:
[clang-tidy] Various improvements in misc-use-override

  * Better error message when more than one of 'virtual', 'override' and 'final'
    is present ("X is/are redundant since the function is already declared Y").
  * Convert the messages to the style used in Clang diagnostics: lower case
    initial letter, no trailing period.
  * Don't run the check for files compiled in pre-C++11 mode
    (http://llvm.org/PR22638).


Added:
    clang-tools-extra/trunk/test/clang-tidy/misc-use-override-cxx98.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp
    clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp
    clang-tools-extra/trunk/test/clang-tidy/misc-use-override.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp?rev=230765&r1=230764&r2=230765&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp Fri Feb 27 10:50:32 2015
@@ -57,6 +57,9 @@ static StringRef GetText(const Token &To
 }
 
 void UseOverride::check(const MatchFinder::MatchResult &Result) {
+  if (!Result.Context->getLangOpts().CPlusPlus11)
+    return;
+
   const FunctionDecl *Method = Result.Nodes.getStmtAs<FunctionDecl>("method");
   const SourceManager &Sources = *Result.SourceManager;
 
@@ -78,11 +81,26 @@ void UseOverride::check(const MatchFinde
   if (!OnlyVirtualSpecified && KeywordCount == 1)
     return; // Nothing to do.
 
-  DiagnosticBuilder Diag = diag(
-      Method->getLocation(),
-      OnlyVirtualSpecified
-          ? "Prefer using 'override' or (rarely) 'final' instead of 'virtual'"
-          : "Annotate this function with 'override' or (rarely) 'final'");
+  std::string Message;
+
+  if (OnlyVirtualSpecified) {
+    Message =
+        "prefer using 'override' or (rarely) 'final' instead of 'virtual'";
+  } else if (KeywordCount == 0) {
+    Message = "annotate this function with 'override' or (rarely) 'final'";
+  } else {
+    StringRef Redundant =
+        HasVirtual ? (HasOverride && HasFinal ? "'virtual' and 'override' are"
+                                              : "'virtual' is")
+                   : "'override' is";
+    StringRef Correct = HasFinal ? "'final'" : "'override'";
+
+    Message =
+        (llvm::Twine(Redundant) +
+         " redundant since the function is already declared " + Correct).str();
+  }
+
+  DiagnosticBuilder Diag = diag(Method->getLocation(), Message);
 
   CharSourceRange FileRange = Lexer::makeFileCharRange(
       CharSourceRange::getTokenRange(Method->getSourceRange()), Sources,
@@ -146,7 +164,7 @@ void UseOverride::check(const MatchFinde
         CharSourceRange::getTokenRange(OverrideLoc, OverrideLoc));
   }
 
-  if (Method->isVirtualAsWritten()) {
+  if (HasVirtual) {
     for (Token Tok : Tokens) {
       if (Tok.is(tok::kw_virtual)) {
         Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(

Modified: clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp?rev=230765&r1=230764&r2=230765&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-diff.cpp Fri Feb 27 10:50:32 2015
@@ -9,10 +9,10 @@ struct A {
 // CHECK-NOT: warning:
 struct B : public A {
   void placeholder_for_f() {}
-// CHECK-SANITY: [[@LINE-1]]:8: warning: Annotate this
-// CHECK: [[@LINE-2]]:8: warning: Annotate this
+// CHECK-SANITY: [[@LINE-1]]:8: warning: annotate this
+// CHECK: [[@LINE-2]]:8: warning: annotate this
   void g() {}
-// CHECK-SANITY: [[@LINE-1]]:8: warning: Annotate this
+// CHECK-SANITY: [[@LINE-1]]:8: warning: annotate this
 // CHECK-NOT: warning:
 };
 // CHECK-SANITY-NOT: Suppressed

Added: clang-tools-extra/trunk/test/clang-tidy/misc-use-override-cxx98.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-use-override-cxx98.cpp?rev=230765&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-use-override-cxx98.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-use-override-cxx98.cpp Fri Feb 27 10:50:32 2015
@@ -0,0 +1,20 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s misc-use-override %t -- -std=c++98
+// REQUIRES: shell
+
+struct Base {
+  virtual ~Base() {}
+  virtual void a();
+  virtual void b();
+};
+
+struct SimpleCases : public Base {
+public:
+  virtual ~SimpleCases();
+  // CHECK-FIXES: {{^}}  virtual ~SimpleCases();
+
+  void a();
+  // CHECK-FIXES: {{^}}  void a();
+
+  virtual void b();
+  // CHECK-FIXES: {{^}}  virtual void b();
+};

Modified: clang-tools-extra/trunk/test/clang-tidy/misc-use-override.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-use-override.cpp?rev=230765&r1=230764&r2=230765&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-use-override.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-use-override.cpp Fri Feb 27 10:50:32 2015
@@ -19,6 +19,7 @@ struct Base {
   virtual void b();
   virtual void c();
   virtual void d();
+  virtual void d2();
   virtual void e() = 0;
   virtual void f() = 0;
   virtual void g() = 0;
@@ -29,17 +30,18 @@ struct Base {
   virtual bool n() MUST_USE_RESULT UNUSED;
 
   virtual void m();
+  virtual void m2();
   virtual void o() __attribute__((unused));
 };
 
 struct SimpleCases : public Base {
 public:
   virtual ~SimpleCases();
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Prefer using 'override' or (rarely) 'final' instead of 'virtual'
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual'
   // CHECK-FIXES: {{^}}  ~SimpleCases() override;
 
   void a();
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this
   // CHECK-FIXES: {{^}}  void a() override;
 
   void b() override;
@@ -47,47 +49,55 @@ public:
   // CHECK-FIXES: {{^}}  void b() override;
 
   virtual void c();
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  void c() override;
 
   virtual void d() override;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'override'
   // CHECK-FIXES: {{^}}  void d() override;
 
+  virtual void d2() final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'final'
+  // CHECK-FIXES: {{^}}  void d2() final;
+
   virtual void e() = 0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  void e() override = 0;
 
   virtual void f()=0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  void f()override =0;
 
   virtual void g() ABSTRACT;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  void g() override ABSTRACT;
 
   virtual void j() const;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  void j() const override;
 
   virtual MustUseResultObject k();  // Has an implicit attribute.
-  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: prefer using
   // CHECK-FIXES: {{^}}  MustUseResultObject k() override;
 
   virtual bool l() MUST_USE_RESULT UNUSED;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  bool l() override MUST_USE_RESULT UNUSED;
 
   virtual bool n() UNUSED MUST_USE_RESULT;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  bool n() override UNUSED MUST_USE_RESULT;
 
-  virtual void m() override final;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Annotate this
+  void m() override final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'override' is redundant since the function is already declared 'final'
   // CHECK-FIXES: {{^}}  void m() final;
 
+  virtual void m2() override final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' and 'override' are redundant since the function is already declared 'final'
+  // CHECK-FIXES: {{^}}  void m2() final;
+
   virtual void o() __attribute__((unused));
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  void o() override __attribute__((unused));
 };
 
@@ -102,14 +112,14 @@ SimpleCases::~SimpleCases() {}
 struct DefaultedDestructor : public Base {
   DefaultedDestructor() {}
   virtual ~DefaultedDestructor() = default;
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using
   // CHECK-FIXES: {{^}}  ~DefaultedDestructor() override = default;
 };
 
 struct FinalSpecified : public Base {
 public:
   virtual ~FinalSpecified() final;
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 'virtual' is redundant since the function is already declared 'final'
   // CHECK-FIXES: {{^}}  ~FinalSpecified() final;
 
   void b() final;
@@ -117,30 +127,30 @@ public:
   // CHECK-FIXES: {{^}}  void b() final;
 
   virtual void d() final;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant
   // CHECK-FIXES: {{^}}  void d() final;
 
   virtual void e() final = 0;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant
   // CHECK-FIXES: {{^}}  void e() final = 0;
 
   virtual void j() const final;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant
   // CHECK-FIXES: {{^}}  void j() const final;
 
   virtual bool l() final MUST_USE_RESULT UNUSED;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant
   // CHECK-FIXES: {{^}}  bool l() final MUST_USE_RESULT UNUSED;
 };
 
 struct InlineDefinitions : public Base {
 public:
   virtual ~InlineDefinitions() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using
   // CHECK-FIXES: {{^}}  ~InlineDefinitions() override {}
 
   void a() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this
   // CHECK-FIXES: {{^}}  void a() override {}
 
   void b() override {}
@@ -148,23 +158,23 @@ public:
   // CHECK-FIXES: {{^}}  void b() override {}
 
   virtual void c() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  void c() override {}
 
   virtual void d() override {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant
   // CHECK-FIXES: {{^}}  void d() override {}
 
   virtual void j() const {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  void j() const override {}
 
   virtual MustUseResultObject k() {}  // Has an implicit attribute.
-  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: prefer using
   // CHECK-FIXES: {{^}}  MustUseResultObject k() override {}
 
   virtual bool l() MUST_USE_RESULT UNUSED {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  bool l() override MUST_USE_RESULT UNUSED {}
 };
 
@@ -172,11 +182,11 @@ struct Macros : public Base {
   // Tests for 'virtual' and 'override' being defined through macros. Basically
   // give up for now.
   NOT_VIRTUAL void a() NOT_OVERRIDE;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: annotate this
   // CHECK-FIXES: {{^}}  NOT_VIRTUAL void a() override NOT_OVERRIDE;
 
   VIRTUAL void b() NOT_OVERRIDE;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  VIRTUAL void b() override NOT_OVERRIDE;
 
   NOT_VIRTUAL void c() OVERRIDE;
@@ -184,7 +194,7 @@ struct Macros : public Base {
   // CHECK-FIXES: {{^}}  NOT_VIRTUAL void c() OVERRIDE;
 
   VIRTUAL void d() OVERRIDE;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant
   // CHECK-FIXES: {{^}}  VIRTUAL void d() OVERRIDE;
 
 #define FUNC(return_type, name) return_type name()
@@ -196,7 +206,7 @@ struct Macros : public Base {
   // CHECK-FIXES: {{^}}  F
 
   VIRTUAL void g() OVERRIDE final;
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Annotate this
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' and 'override' are redundant
   // CHECK-FIXES: {{^}}  VIRTUAL void g() final;
 };
 
@@ -207,7 +217,7 @@ template <typename T> struct TemplateBas
 
 template <typename T> struct DerivedFromTemplate : public TemplateBase<T> {
   virtual void f(T t);
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  void f(T t) override;
 };
 void f() { DerivedFromTemplate<int>().f(2); }
@@ -215,7 +225,7 @@ void f() { DerivedFromTemplate<int>().f(
 template <class C>
 struct UnusedMemberInstantiation : public C {
   virtual ~UnusedMemberInstantiation() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Prefer using
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using
   // CHECK-FIXES: {{^}}  ~UnusedMemberInstantiation() override {}
 };
 struct IntantiateWithoutUse : public UnusedMemberInstantiation<Base> {};





More information about the cfe-commits mailing list