Hi alexfh,
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.
http://reviews.llvm.org/D9286
Files:
clang-tidy/misc/UseOverrideCheck.cpp
test/clang-tidy/misc-use-override-keep-virtual.cpp
test/clang-tidy/misc-use-override.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: clang-tidy/misc/UseOverrideCheck.cpp
===================================================================
--- clang-tidy/misc/UseOverrideCheck.cpp
+++ clang-tidy/misc/UseOverrideCheck.cpp
@@ -148,8 +148,30 @@
}
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.
+ int Parens = 0;
+ bool Found = false;
+ for (Token T : Tokens) {
+ if (T.is(tok::l_paren))
+ ++Parens;
+ else if (T.is(tok::r_paren) && --Parens == 0) {
+ InsertLoc = T.getEndLoc();
+ ReplacementText = " override";
+ Found = true;
+ }
+ else if (Found && (T.is(tok::kw_const) ||
+ T.is(tok::kw_volatile) ||
+ T.is(tok::amp) ||
+ T.is(tok::ampamp)))
+ InsertLoc = T.getEndLoc();
+ else if (T.is(tok::l_brace))
+ break;
+ }
+ }
if (!InsertLoc.isValid()) {
// For declarations marked with "= 0" or "= [default|delete]", the end
Index: test/clang-tidy/misc-use-override-keep-virtual.cpp
===================================================================
--- test/clang-tidy/misc-use-override-keep-virtual.cpp
+++ test/clang-tidy/misc-use-override-keep-virtual.cpp
@@ -33,6 +33,11 @@
virtual void m();
virtual void m2();
virtual void o() __attribute__((unused));
+
+ virtual void r() &;
+ virtual void rr() &&;
+
+ virtual void cv() const volatile;
};
struct SimpleCases : public Base {
@@ -158,25 +163,42 @@
// CHECK-MESSAGES-NOT: warning:
// CHECK-FIXES: {{^}} void b() override {}
- virtual void c() {}
- // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
- // CHECK-FIXES: {{^}} virtual void c() override {}
+ virtual void c()
+ {}
+ // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: prefer using
+ // CHECK-FIXES: {{^}} virtual void c() override
virtual void d() override {}
// CHECK-MESSAGES-NOT: warning:
// CHECK-FIXES: {{^}} virtual void d() override {}
- virtual void j() const {}
- // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
- // CHECK-FIXES: {{^}} virtual void j() const override {}
+ virtual void j() const
+ {}
+ // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: prefer using
+ // CHECK-FIXES: {{^}} virtual void j() const override
virtual MustUseResultObject k() {} // Has an implicit attribute.
// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: prefer using
// CHECK-FIXES: {{^}} virtual MustUseResultObject k() override {}
virtual bool l() MUST_USE_RESULT UNUSED {}
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
// CHECK-FIXES: {{^}} virtual bool l() override MUST_USE_RESULT UNUSED {}
+
+ virtual void r() &
+ {}
+ // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: prefer using
+ // CHECK-FIXES: {{^}} virtual void r() & override
+
+ virtual void rr() &&
+ {}
+ // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: prefer using
+ // CHECK-FIXES: {{^}} virtual void rr() && override
+
+ virtual void cv() const volatile
+ {}
+ // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: prefer using
+ // CHECK-FIXES: {{^}} virtual void cv() const volatile override
};
struct Macros : public Base {
Index: test/clang-tidy/misc-use-override.cpp
===================================================================
--- test/clang-tidy/misc-use-override.cpp
+++ test/clang-tidy/misc-use-override.cpp
@@ -32,6 +32,11 @@
virtual void m();
virtual void m2();
virtual void o() __attribute__((unused));
+
+ virtual void r() &;
+ virtual void rr() &&;
+
+ virtual void cv() const volatile;
};
struct SimpleCases : public Base {
@@ -157,25 +162,42 @@
// 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
// CHECK-FIXES: {{^}} MustUseResultObject k() override {}
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
};
struct Macros : public Base {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
