<div dir="ltr"><div>+static SmallVector<Token, 16> ParseTokens(CharSourceRange Range,</div><div>+ const SourceManager &Sources,</div><div>+ LangOptions LangOpts) {</div>
<div><br></div><div>+static StringRef GetText(const Token& Tok, const SourceManager &Sources) {</div><div><br></div><div>Naming.</div><div><br></div><div>+ for (const clang::Attr *attr : Method->getAttrs()) {</div>
<div><br></div><div>Naming. Maybe just use "A"?</div><div><br></div><div>+ for (unsigned i = 0, e = Tokens.size(); i != e; ++i) {</div><div>+ if (Tokens[i].is(tok::raw_identifier) &&</div><div>+ GetText(Tokens[i], Sources) == "virtual") {</div>
<div>+ Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(</div><div>+ Tokens[i].getLocation(), Tokens[i].getLocation()));</div><div>+ break;</div><div>+ }</div><div>+ }</div>
<div><br></div><div>Can this be range-ified?</div><div><br></div><div>-- Sean Silva</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, May 16, 2014 at 3:30 AM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: djasper<br>
Date: Fri May 16 04:30:09 2014<br>
New Revision: 208954<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=208954&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=208954&view=rev</a><br>
Log:<br>
Initial version of clang-tidy check to use override instead of virual.<br>
<br>
Review: <a href="http://reviews.llvm.org/D3688" target="_blank">http://reviews.llvm.org/D3688</a><br>
<br>
Added:<br>
clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp<br>
clang-tools-extra/trunk/clang-tidy/misc/UseOverride.h<br>
clang-tools-extra/trunk/test/clang-tidy/use-override.cpp<br>
Modified:<br>
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt<br>
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp<br>
clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=208954&r1=208953&r2=208954&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=208954&r1=208953&r2=208954&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Fri May 16 04:30:09 2014<br>
@@ -4,6 +4,7 @@ add_clang_library(clangTidyMiscModule<br>
ArgumentCommentCheck.cpp<br>
MiscTidyModule.cpp<br>
RedundantSmartptrGet.cpp<br>
+ UseOverride.cpp<br>
<br>
LINK_LIBS<br>
clangAST<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=208954&r1=208953&r2=208954&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=208954&r1=208953&r2=208954&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Fri May 16 04:30:09 2014<br>
@@ -12,6 +12,7 @@<br>
#include "../ClangTidyModuleRegistry.h"<br>
#include "ArgumentCommentCheck.h"<br>
#include "RedundantSmartptrGet.h"<br>
+#include "UseOverride.h"<br>
<br>
namespace clang {<br>
namespace tidy {<br>
@@ -25,6 +26,9 @@ public:<br>
CheckFactories.addCheckFactory(<br>
"misc-redundant-smartptr-get",<br>
new ClangTidyCheckFactory<RedundantSmartptrGet>());<br>
+ CheckFactories.addCheckFactory(<br>
+ "misc-use-override",<br>
+ new ClangTidyCheckFactory<UseOverride>());<br>
}<br>
};<br>
<br>
<br>
Added: clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp?rev=208954&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp?rev=208954&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp (added)<br>
+++ clang-tools-extra/trunk/clang-tidy/misc/UseOverride.cpp Fri May 16 04:30:09 2014<br>
@@ -0,0 +1,131 @@<br>
+//===--- UseOverride.cpp - clang-tidy -------------------------------------===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#include "UseOverride.h"<br>
+#include "clang/AST/ASTContext.h"<br>
+#include "clang/ASTMatchers/ASTMatchFinder.h"<br>
+#include "clang/Lex/Lexer.h"<br>
+<br>
+using namespace clang::ast_matchers;<br>
+<br>
+namespace clang {<br>
+namespace tidy {<br>
+<br>
+void UseOverride::registerMatchers(MatchFinder *Finder) {<br>
+ Finder->addMatcher(methodDecl(isOverride()).bind("method"), this);<br>
+}<br>
+<br>
+// Re-lex the tokens to get precise locations to insert 'override' and remove<br>
+// 'virtual'.<br>
+static SmallVector<Token, 16> ParseTokens(CharSourceRange Range,<br>
+ const SourceManager &Sources,<br>
+ LangOptions LangOpts) {<br>
+ std::pair<FileID, unsigned> LocInfo =<br>
+ Sources.getDecomposedLoc(Range.getBegin());<br>
+ StringRef File = Sources.getBufferData(LocInfo.first);<br>
+ const char *TokenBegin = File.data() + LocInfo.second;<br>
+ Lexer RawLexer(Sources.getLocForStartOfFile(LocInfo.first), LangOpts,<br>
+ File.begin(), TokenBegin, File.end());<br>
+ SmallVector<Token, 16> Tokens;<br>
+ Token Tok;<br>
+ while (!RawLexer.LexFromRawLexer(Tok)) {<br>
+ if (Tok.is(tok::semi) || Tok.is(tok::l_brace))<br>
+ break;<br>
+ if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation()))<br>
+ break;<br>
+ Tokens.push_back(Tok);<br>
+ }<br>
+ return Tokens;<br>
+}<br>
+<br>
+static StringRef GetText(const Token& Tok, const SourceManager &Sources) {<br>
+ return {Sources.getCharacterData(Tok.getLocation()), Tok.getLength()};<br>
+}<br>
+<br>
+void UseOverride::check(const MatchFinder::MatchResult &Result) {<br>
+ const FunctionDecl *Method = Result.Nodes.getStmtAs<FunctionDecl>("method");<br>
+ const SourceManager &Sources = *Result.SourceManager;<br>
+<br>
+ assert(Method != nullptr);<br>
+ if (Method->getInstantiatedFromMemberFunction() != nullptr)<br>
+ Method = Method->getInstantiatedFromMemberFunction();<br>
+<br>
+ if (Method->isImplicit() || Method->getLocation().isMacroID() ||<br>
+ Method->isOutOfLine())<br>
+ return;<br>
+<br>
+ if (Method->getAttr<clang::OverrideAttr>() != nullptr &&<br>
+ !Method->isVirtualAsWritten())<br>
+ return; // Nothing to do.<br>
+<br>
+ DiagnosticBuilder Diag = diag(Method->getLocation(),<br>
+ "Prefer using 'override' instead of 'virtual'");<br>
+<br>
+ CharSourceRange FileRange =<br>
+ Lexer::makeFileCharRange(CharSourceRange::getTokenRange(<br>
+ Method->getLocStart(), Method->getLocEnd()),<br>
+ Sources, Result.Context->getLangOpts());<br>
+<br>
+ if (!FileRange.isValid())<br>
+ return;<br>
+<br>
+ // FIXME: Instead of re-lexing and looking for specific macros such as<br>
+ // 'ABSTRACT', properly store the location of 'virtual' and '= 0' in each<br>
+ // FunctionDecl.<br>
+ SmallVector<Token, 16> Tokens = ParseTokens(FileRange, Sources,<br>
+ Result.Context->getLangOpts());<br>
+<br>
+ // Add 'override' on inline declarations that don't already have it.<br>
+ if (Method->getAttr<clang::OverrideAttr>() == nullptr) {<br>
+ SourceLocation InsertLoc;<br>
+ StringRef ReplacementText = "override ";<br>
+<br>
+ if (Method->hasAttrs()) {<br>
+ for (const clang::Attr *attr : Method->getAttrs()) {<br>
+ if (!attr->isImplicit()) {<br>
+ InsertLoc = Sources.getExpansionLoc(attr->getLocation());<br>
+ break;<br>
+ }<br>
+ }<br>
+ }<br>
+<br>
+ if (InsertLoc.isInvalid() && Method->doesThisDeclarationHaveABody()) {<br>
+ InsertLoc = Method->getBody()->getLocStart();<br>
+ }<br>
+<br>
+ if (!InsertLoc.isValid()) {<br>
+ if (Tokens.size() > 2 && GetText(Tokens.back(), Sources) == "0" &&<br>
+ GetText(Tokens[Tokens.size() - 2], Sources) == "=") {<br>
+ InsertLoc = Tokens[Tokens.size() - 2].getLocation();<br>
+ } else if (GetText(Tokens.back(), Sources) == "ABSTRACT") {<br>
+ InsertLoc = Tokens.back().getLocation();<br>
+ }<br>
+ }<br>
+<br>
+ if (!InsertLoc.isValid()) {<br>
+ InsertLoc = FileRange.getEnd();<br>
+ ReplacementText = " override";<br>
+ }<br>
+ Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText);<br>
+ }<br>
+<br>
+ if (Method->isVirtualAsWritten()) {<br>
+ for (unsigned i = 0, e = Tokens.size(); i != e; ++i) {<br>
+ if (Tokens[i].is(tok::raw_identifier) &&<br>
+ GetText(Tokens[i], Sources) == "virtual") {<br>
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(<br>
+ Tokens[i].getLocation(), Tokens[i].getLocation()));<br>
+ break;<br>
+ }<br>
+ }<br>
+ }<br>
+}<br>
+<br>
+} // namespace tidy<br>
+} // namespace clang<br>
<br>
Added: clang-tools-extra/trunk/clang-tidy/misc/UseOverride.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UseOverride.h?rev=208954&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UseOverride.h?rev=208954&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/misc/UseOverride.h (added)<br>
+++ clang-tools-extra/trunk/clang-tidy/misc/UseOverride.h Fri May 16 04:30:09 2014<br>
@@ -0,0 +1,29 @@<br>
+//===--- UseOverride.h - clang-tidy -----------------------------*- C++ -*-===//<br>
+//<br>
+// The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_H<br>
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_H<br>
+<br>
+#include "../ClangTidy.h"<br>
+<br>
+namespace clang {<br>
+namespace tidy {<br>
+<br>
+/// \brief Use C++11's 'override' and remove 'virtual' where applicable.<br>
+class UseOverride : public ClangTidyCheck {<br>
+public:<br>
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;<br>
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;<br>
+};<br>
+<br>
+} // namespace tidy<br>
+} // namespace clang<br>
+<br>
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_H<br>
+<br>
<br>
Modified: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh?rev=208954&r1=208953&r2=208954&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh?rev=208954&r1=208953&r2=208954&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh (original)<br>
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh Fri May 16 04:30:09 2014<br>
@@ -8,4 +8,4 @@ TEMPORARY_FILE=$3.cpp<br>
<br>
grep -Ev "// *[A-Z-]+:" ${INPUT_FILE} > ${TEMPORARY_FILE}<br>
clang-tidy ${TEMPORARY_FILE} -fix --checks="-*,${CHECK_TO_RUN}" -- --std=c++11<br>
-FileCheck -input-file=${TEMPORARY_FILE} ${INPUT_FILE}<br>
+FileCheck -input-file=${TEMPORARY_FILE} ${INPUT_FILE} -strict-whitespace<br>
<br>
Added: clang-tools-extra/trunk/test/clang-tidy/use-override.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/use-override.cpp?rev=208954&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/use-override.cpp?rev=208954&view=auto</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/test/clang-tidy/use-override.cpp (added)<br>
+++ clang-tools-extra/trunk/test/clang-tidy/use-override.cpp Fri May 16 04:30:09 2014<br>
@@ -0,0 +1,134 @@<br>
+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-use-override %t<br>
+// REQUIRES: shell<br>
+<br>
+#define ABSTRACT = 0<br>
+<br>
+#define OVERRIDE override<br>
+#define VIRTUAL virtual<br>
+#define NOT_VIRTUAL<br>
+#define NOT_OVERRIDE<br>
+<br>
+#define MUST_USE_RESULT __attribute__((warn_unused_result))<br>
+<br>
+struct MUST_USE_RESULT MustUseResultObject {};<br>
+<br>
+struct Base {<br>
+ virtual ~Base() {}<br>
+ virtual void a();<br>
+ virtual void b();<br>
+ virtual void c();<br>
+ virtual void d();<br>
+ virtual void e() = 0;<br>
+ virtual void f() = 0;<br>
+ virtual void g() = 0;<br>
+<br>
+ virtual void j() const;<br>
+ virtual MustUseResultObject k();<br>
+ virtual bool l() MUST_USE_RESULT;<br>
+};<br>
+<br>
+struct SimpleCases : public Base {<br>
+public:<br>
+ virtual ~SimpleCases();<br>
+ // CHECK: {{^ ~SimpleCases\(\) override;}}<br>
+<br>
+ void a();<br>
+ // CHECK: {{^ void a\(\) override;}}<br>
+ void b() override;<br>
+ // CHECK: {{^ void b\(\) override;}}<br>
+ virtual void c();<br>
+ // CHECK: {{^ void c\(\) override;}}<br>
+ virtual void d() override;<br>
+ // CHECK: {{^ void d\(\) override;}}<br>
+<br>
+ virtual void e() = 0;<br>
+ // CHECK: {{^ void e\(\) override = 0;}}<br>
+ virtual void f()=0;<br>
+ // CHECK: {{^ void f\(\)override =0;}}<br>
+ virtual void g() ABSTRACT;<br>
+ // CHECK: {{^ void g\(\) override ABSTRACT;}}<br>
+<br>
+ virtual void j() const;<br>
+ // CHECK: {{^ void j\(\) const override;}}<br>
+ virtual MustUseResultObject k(); // Has an implicit attribute.<br>
+ // CHECK: {{^ MustUseResultObject k\(\) override;}}<br>
+ virtual bool l() MUST_USE_RESULT; // Has an explicit attribute<br>
+ // CHECK: {{^ bool l\(\) override MUST_USE_RESULT;}}<br>
+};<br>
+<br>
+void SimpleCases::i() {}<br>
+// CHECK: {{^void SimpleCases::i\(\) {}}}<br>
+<br>
+struct InlineDefinitions : public Base {<br>
+public:<br>
+ virtual ~InlineDefinitions() {}<br>
+ // CHECK: {{^ ~InlineDefinitions\(\) override {}}}<br>
+<br>
+ void a() {}<br>
+ // CHECK: {{^ void a\(\) override {}}}<br>
+ void b() override {}<br>
+ // CHECK: {{^ void b\(\) override {}}}<br>
+ virtual void c() {}<br>
+ // CHECK: {{^ void c\(\) override {}}}<br>
+ virtual void d() override {}<br>
+ // CHECK: {{^ void d\(\) override {}}}<br>
+<br>
+ virtual void j() const {}<br>
+ // CHECK: {{^ void j\(\) const override {}}}<br>
+ virtual MustUseResultObject k() {} // Has an implicit attribute.<br>
+ // CHECK: {{^ MustUseResultObject k\(\) override {}}}<br>
+ virtual bool l() MUST_USE_RESULT {} // Has an explicit attribute<br>
+ // CHECK: {{^ bool l\(\) override MUST_USE_RESULT {}}}<br>
+};<br>
+<br>
+struct Macros : public Base {<br>
+ // Tests for 'virtual' and 'override' being defined through macros. Basically<br>
+ // give up for now.<br>
+ NOT_VIRTUAL void a() NOT_OVERRIDE;<br>
+ // CHECK: {{^ NOT_VIRTUAL void a\(\) override NOT_OVERRIDE;}}<br>
+<br>
+ VIRTUAL void b() NOT_OVERRIDE;<br>
+ // CHECK: {{^ VIRTUAL void b\(\) override NOT_OVERRIDE;}}<br>
+<br>
+ NOT_VIRTUAL void c() OVERRIDE;<br>
+ // CHECK: {{^ NOT_VIRTUAL void c\(\) OVERRIDE;}}<br>
+<br>
+ VIRTUAL void d() OVERRIDE;<br>
+ // CHECK: {{^ VIRTUAL void d\(\) OVERRIDE;}}<br>
+<br>
+#define FUNC(name, return_type) return_type name()<br>
+ FUNC(void, e);<br>
+ // CHECK: {{^ FUNC\(void, e\);}}<br>
+<br>
+#define F virtual void f();<br>
+ F<br>
+ // CHECK: {{^ F}}<br>
+};<br>
+<br>
+// Tests for templates.<br>
+template <typename T> struct TemplateBase {<br>
+ virtual void f(T t);<br>
+};<br>
+<br>
+template <typename T> struct DerivedFromTemplate : public TemplateBase<T> {<br>
+ virtual void f(T t);<br>
+ // CHECK: {{^ void f\(T t\) override;}}<br>
+};<br>
+void f() { DerivedFromTemplate<int>().f(2); }<br>
+<br>
+template <class C><br>
+struct UnusedMemberInstantiation : public C {<br>
+ virtual ~UnusedMemberInstantiation() {}<br>
+ // CHECK: {{^ ~UnusedMemberInstantiation\(\) override {}}}<br>
+};<br>
+struct IntantiateWithoutUse : public UnusedMemberInstantiation<Base> {};<br>
+<br>
+// The OverrideAttr isn't propagated to specializations in all cases. Make sure<br>
+// we don't add "override" a second time.<br>
+template <int I><br>
+struct MembersOfSpecializations : public Base {<br>
+ void a() override;<br>
+ // CHECK: {{^ void a\(\) override;}}<br>
+};<br>
+template <> void MembersOfSpecializations<3>::a() {}<br>
+void f() { D<3>().a(); };<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>