[clang-tools-extra] r245401 - Insert override at the same line as the end of the function declaration

[Ehsan Akhgari via cfe-commits]

Author: ehsan
Date: Tue Aug 18 21:05:37 2015
New Revision: 245401

URL: http://llvm.org/viewvc/llvm-project?rev=245401&view=rev
Log:
Insert override at the same line as the end of the function declaration

Summary:
The existing check converts the code pattern below:

  void f()
  {
  }

to:

  void f()
  override {
  }

which is fairly sub-optimal.  This patch fixes this by inserting the
override keyword on the same line as the function declaration if
possible, so that we instead get:

  void f() override
  {
  }

We do this by looking for the last token before the start of the body
and inserting the override keyword at the end of its location.  Note
that we handle const, volatile and ref-qualifiers correctly.

Test Plan: Includes an automated test.

Reviewers: alexfh

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D9286

Modified:
    clang-tools-extra/trunk/clang-tidy/misc/UseOverrideCheck.cpp
    clang-tools-extra/trunk/test/clang-tidy/misc-use-override.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/UseOverrideCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UseOverrideCheck.cpp?rev=245401&r1=245400&r2=245401&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/UseOverrideCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UseOverrideCheck.cpp Tue Aug 18 21:05:37 2015
@@ -140,8 +140,20 @@ void UseOverrideCheck::check(const Match
     }
 
     if (InsertLoc.isInvalid() && Method->doesThisDeclarationHaveABody() &&
-        Method->getBody() && !Method->isDefaulted())
-      InsertLoc = Method->getBody()->getLocStart();
+        Method->getBody() && !Method->isDefaulted()) {
+      // For methods with inline definition, add the override keyword at the
+      // end of the declaration of the function, but prefer to put it on the
+      // same line as the declaration if the beginning brace for the start of
+      // the body falls on the next line.
+      Token LastNonCommentToken;
+      for (Token T : Tokens) {
+        if (!T.is(tok::comment)) {
+          LastNonCommentToken = T;
+        }
+      }
+      InsertLoc = LastNonCommentToken.getEndLoc();
+      ReplacementText = " override";
+    }
 
     if (!InsertLoc.isValid()) {
       // For declarations marked with "= 0" or "= [default|delete]", the end

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=245401&r1=245400&r2=245401&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 Tue Aug 18 21:05:37 2015
@@ -32,6 +32,12 @@ struct Base {
   virtual void m();
   virtual void m2();
   virtual void o() __attribute__((unused));
+
+  virtual void r() &;
+  virtual void rr() &&;
+
+  virtual void cv() const volatile;
+  virtual void cv2() const volatile;
 };
 
 struct SimpleCases : public Base {
@@ -157,17 +163,19 @@ public:
   // CHECK-MESSAGES-NOT: warning:
   // CHECK-FIXES: {{^}}  void b() override {}
 
-  virtual void c() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
-  // CHECK-FIXES: {{^}}  void c() override {}
+  virtual void c()
+  {}
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void c() override
 
   virtual void d() override {}
   // 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-FIXES: {{^}}  void j() const override {}
+  virtual void j() const
+  {}
+  // CHECK-MESSAGES: :[[@LINE-2]]: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
@@ -176,6 +184,26 @@ public:
   virtual bool l() MUST_USE_RESULT UNUSED {}
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
   // CHECK-FIXES: {{^}}  bool l() override MUST_USE_RESULT UNUSED {}
+
+  virtual void r() &
+  {}
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void r() & override
+
+  virtual void rr() &&
+  {}
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void rr() && override
+
+  virtual void cv() const volatile
+  {}
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void cv() const volatile override
+
+  virtual void cv2() const volatile // some comment
+  {}
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void cv2() const volatile override // some comment
 };
 
 struct Macros : public Base {