Author: alexfh
Date: Mon Apr 4 09:31:36 2016
New Revision: 265298
URL: http://llvm.org/viewvc/llvm-project?rev=265298&view=rev
Log:
[clang-tidy] fix a couple of modernize-use-override bugs
Fix for __declspec attributes and const=0 without space
This patch is to address 2 problems I found with
Clang-tidy:modernize-use-override.
1: missing spaces on pure function decls.
Orig:
void pure() const=0
Problem:
void pure() constoverride =0
Fixed:
void pure() const override =0
2: This is ms-extension specific, but possibly applies to other attribute
types. The override is placed before the attribute which doesn’t work well with
declspec as this attribute can be inherited or placed before the method
identifier.
Orig:
class __declspec(dllexport) X : public Y
{
void p();
};
Problem:
class override __declspec(dllexport) class X : public Y
{
void p();
};
Fixed:
class __declspec(dllexport) class X : public Y
{
void p() override;
};
Patch by Robert Bolter!
Differential Revision: http://reviews.llvm.org/D18396
Added:
clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-ms.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/test/clang-tidy/modernize-use-override.cpp
Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp?rev=265298&r1=265297&r2=265298&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp Mon Apr
4 09:31:36 2016
@@ -95,9 +95,9 @@ void UseOverrideCheck::check(const Match
: "'override' is";
StringRef Correct = HasFinal ? "'final'" : "'override'";
- Message =
- (llvm::Twine(Redundant) +
- " redundant since the function is already declared " + Correct).str();
+ Message = (llvm::Twine(Redundant) +
+ " redundant since the function is already declared " + Correct)
+ .str();
}
DiagnosticBuilder Diag = diag(Method->getLocation(), Message);
@@ -118,9 +118,11 @@ void UseOverrideCheck::check(const Match
if (!HasFinal && !HasOverride) {
SourceLocation InsertLoc;
StringRef ReplacementText = "override ";
+ SourceLocation MethodLoc = Method->getLocation();
for (Token T : Tokens) {
- if (T.is(tok::kw___attribute)) {
+ if (T.is(tok::kw___attribute) &&
+ !Sources.isBeforeInTranslationUnit(T.getLocation(), MethodLoc)) {
InsertLoc = T.getLocation();
break;
}
@@ -128,11 +130,12 @@ void UseOverrideCheck::check(const Match
if (Method->hasAttrs()) {
for (const clang::Attr *A : Method->getAttrs()) {
- if (!A->isImplicit()) {
+ if (!A->isImplicit() && !A->isInherited()) {
SourceLocation Loc =
Sources.getExpansionLoc(A->getRange().getBegin());
- if (!InsertLoc.isValid() ||
- Sources.isBeforeInTranslationUnit(Loc, InsertLoc))
+ if ((!InsertLoc.isValid() ||
+ Sources.isBeforeInTranslationUnit(Loc, InsertLoc)) &&
+ !Sources.isBeforeInTranslationUnit(Loc, MethodLoc))
InsertLoc = Loc;
}
}
@@ -163,6 +166,9 @@ void UseOverrideCheck::check(const Match
Tokens.back().is(tok::kw_delete)) &&
GetText(Tokens[Tokens.size() - 2], Sources) == "=") {
InsertLoc = Tokens[Tokens.size() - 2].getLocation();
+ // Check if we need to insert a space.
+ if ((Tokens[Tokens.size() - 2].getFlags() & Token::LeadingSpace) == 0)
+ ReplacementText = " override ";
} else if (GetText(Tokens.back(), Sources) == "ABSTRACT") {
InsertLoc = Tokens.back().getLocation();
}
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=265298&r1=265297&r2=265298&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Mon Apr 4 09:31:36 2016
@@ -155,9 +155,12 @@ identified. The improvements since the
Fixed bugs:
- Crash when running on compile database with relative source files paths.
+- Crash when running on compile database with relative source files paths.
- Crash when running with the `-fdelayed-template-parsing` flag.
+- Crash when running with the `-fdelayed-template-parsing` flag.
+
+- The ``modernize-use-override`` check: incorrect fix-its placement around
+ ``__declspec`` and other attributes.
Clang-tidy changes from 3.7 to 3.8
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Added: clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-ms.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-ms.cpp?rev=265298&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-ms.cpp
(added)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-ms.cpp Mon
Apr 4 09:31:36 2016
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- -- -fms-extensions
-std=c++11
+
+// This test is designed to test ms-extension __declspec(dllexport) attributes.
+#define EXPORT __declspec(dllexport)
+
+class Base {
+ virtual EXPORT void a();
+};
+
+class EXPORT InheritedBase {
+ virtual void a();
+};
+
+class Derived : public Base {
+ virtual EXPORT void a();
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: prefer using 'override' or
(rarely) 'final' instead of 'virtual' [modernize-use-override]
+ // CHECK-FIXES: {{^}} EXPORT void a() override;
+};
+
+class EXPORT InheritedDerived : public InheritedBase {
+ virtual void a();
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using 'override' or
(rarely) 'final' instead of 'virtual' [modernize-use-override]
+ // CHECK-FIXES: {{^}} void a() override;
+};
+
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-use-override.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-override.cpp?rev=265298&r1=265297&r2=265298&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-override.cpp
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-override.cpp Mon Apr
4 09:31:36 2016
@@ -21,6 +21,7 @@ struct Base {
virtual void d2();
virtual void e() = 0;
virtual void f() = 0;
+ virtual void f2() const = 0;
virtual void g() = 0;
virtual void j() const;
@@ -74,7 +75,11 @@ public:
virtual void f()=0;
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
- // CHECK-FIXES: {{^}} void f()override =0;
+ // CHECK-FIXES: {{^}} void f() override =0;
+
+ virtual void f2() const=0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+ // CHECK-FIXES: {{^}} void f2() const override =0;
virtual void g() ABSTRACT;
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
