Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -4,6 +4,7 @@ ArgumentCommentCheck.cpp MiscTidyModule.cpp RedundantSmartptrGet.cpp + UseOverride.cpp LINK_LIBS clangAST Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -12,6 +12,7 @@ #include "../ClangTidyModuleRegistry.h" #include "ArgumentCommentCheck.h" #include "RedundantSmartptrGet.h" +#include "UseOverride.h" namespace clang { namespace tidy { @@ -25,6 +26,9 @@ CheckFactories.addCheckFactory( "misc-redundant-smartptr-get", new ClangTidyCheckFactory()); + CheckFactories.addCheckFactory( + "misc-use-override", + new ClangTidyCheckFactory()); } }; Index: clang-tidy/misc/UseOverride.h =================================================================== --- /dev/null +++ clang-tidy/misc/UseOverride.h @@ -0,0 +1,29 @@ +//===--- UseOverride.h - clang-tidy -----------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// \brief Use C++11's 'override' and remove 'virtual' where applicable. +class UseOverride : public ClangTidyCheck { +public: + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USE_OVERRIDE_H + Index: clang-tidy/misc/UseOverride.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/UseOverride.cpp @@ -0,0 +1,120 @@ +//===--- UseOverride.cpp - clang-tidy -------------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "UseOverride.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { + +void UseOverride::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(methodDecl(isOverride()).bind("method"), this); +} + +void UseOverride::check(const MatchFinder::MatchResult &Result) { + const FunctionDecl *Method = Result.Nodes.getStmtAs("method"); + assert(Method != nullptr); + if (Method->getInstantiatedFromMemberFunction() != nullptr) + Method = Method->getInstantiatedFromMemberFunction(); + + if (Method->isImplicit() || Method->getLocation().isMacroID() || + Method->isOutOfLine()) + return; + + if (Method->getAttr() != nullptr && + !Method->isVirtualAsWritten()) + return; // Nothing to do. + + DiagnosticBuilder Diag = diag(Method->getLocation(), + "Prefer using 'override' instead of 'virtual'"); + + CharSourceRange FileRange = Lexer::makeFileCharRange( + CharSourceRange::getTokenRange(Method->getLocStart(), + Method->getLocEnd()), + *Result.SourceManager, Result.Context->getLangOpts()); + + if (!FileRange.isValid()) + return; + + // Re-lex the tokens to get precise locations to insert 'override' and remove + // 'virtual'. + std::pair LocInfo = + Result.SourceManager->getDecomposedLoc(FileRange.getBegin()); + StringRef File = Result.SourceManager->getBufferData(LocInfo.first); + const char *TokenBegin = File.data() + LocInfo.second; + Lexer RawLexer(Result.SourceManager->getLocForStartOfFile(LocInfo.first), + Result.Context->getLangOpts(), File.begin(), TokenBegin, + File.end()); + SmallVector Tokens; + SmallVector TokenText; + Token Tok; + while (!RawLexer.LexFromRawLexer(Tok)) { + if (Tok.is(tok::semi) || Tok.is(tok::l_brace)) + break; + if (Result.SourceManager->isBeforeInTranslationUnit(FileRange.getEnd(), + Tok.getLocation())) + break; + Tokens.push_back(Tok); + TokenText.push_back( + StringRef(Result.SourceManager->getCharacterData(Tok.getLocation()), + Tok.getLength())); + } + + // Add 'override' on inline declarations that don't already have it. + if (Method->getAttr() == nullptr) { + SourceLocation InsertLoc; + StringRef ReplacementText = "override "; + + if (Method->hasAttrs()) { + for (const clang::Attr *attr : Method->getAttrs()) { + if (!attr->isImplicit()) { + InsertLoc = + Result.SourceManager->getExpansionLoc(attr->getLocation()); + break; + } + } + } + + if (InsertLoc.isInvalid() && Method->doesThisDeclarationHaveABody()) { + InsertLoc = Method->getBody()->getLocStart(); + } + + if (!InsertLoc.isValid()) { + if (Tokens.size() > 2 && TokenText.back() == "0" && + TokenText[Tokens.size() - 2] == "=") { + InsertLoc = Tokens[Tokens.size() - 2].getLocation(); + } else if (TokenText.back() == "ABSTRACT") { + InsertLoc = Tokens.back().getLocation(); + } + } + + if (!InsertLoc.isValid()) { + InsertLoc = FileRange.getEnd(); + ReplacementText = " override"; + } + Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText); + } + + if (Method->isVirtualAsWritten()) { + for (unsigned i = 0, e = Tokens.size(); i != e; ++i) { + if (Tokens[i].is(tok::raw_identifier) && TokenText[i] == "virtual") { + Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( + Tokens[i].getLocation(), Tokens[i].getLocation())); + break; + } + } + } +} + +} // namespace tidy +} // namespace clang Index: test/clang-tidy/check_clang_tidy_fix.sh =================================================================== --- test/clang-tidy/check_clang_tidy_fix.sh +++ test/clang-tidy/check_clang_tidy_fix.sh @@ -9,4 +9,4 @@ grep -Ev "// *[A-Z-]+:" ${INPUT_FILE} > ${TEMPORARY_FILE} clang-tidy ${TEMPORARY_FILE} -fix --checks=${CHECK_TO_RUN} \ --disable-checks="" -- --std=c++11 -FileCheck -input-file=${TEMPORARY_FILE} ${INPUT_FILE} +FileCheck -input-file=${TEMPORARY_FILE} ${INPUT_FILE} -strict-whitespace Index: test/clang-tidy/use-override.cpp =================================================================== --- /dev/null +++ test/clang-tidy/use-override.cpp @@ -0,0 +1,65 @@ +// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-use-override %t +// REQUIRES: shell + +#define ABSTRACT = 0 + +#define OVERRIDE override +#define VIRTUAL virtual +#define NOT_VIRTUAL +#define NOT_OVERRIDE + +struct Base { + virtual void a(); + virtual void b(); + virtual void c(); + virtual void d(); + virtual void e() = 0; + virtual void f() = 0; + virtual void g() = 0; + virtual void h(); + virtual void i(); +}; + +struct SimpleCases : public Base { +public: + void a(); + // CHECK: {{^ void a\(\) override;}} + void b() override; + // CHECK: {{^ void b\(\) override;}} + virtual void c(); + // CHECK: {{^ void c\(\) override;}} + virtual void d() override; + // CHECK: {{^ void d\(\) override;}} + + virtual void e() = 0; + // CHECK: {{^ void e\(\) override = 0;}} + virtual void f()=0; + // CHECK: {{^ void f\(\)override =0;}} + virtual void g() ABSTRACT; + // CHECK: {{^ void g\(\) override ABSTRACT;}} + + virtual void h() {} + // CHECK: {{^ void h\(\) override {}}} + + virtual void i(); + // CHECK: {{^ void i\(\) override;}} +}; + +void SimpleCases::i() {} +// CHECK: {{^void SimpleCases::i\(\) {}}} + +// Tests for 'virtual' and 'override' being defined through macros. Basically +// give up for now. +struct Macros : public Base { + NOT_VIRTUAL void a() NOT_OVERRIDE; + // CHECK: {{^ NOT_VIRTUAL void a\(\) override NOT_OVERRIDE;}} + + VIRTUAL void b() NOT_OVERRIDE; + // CHECK: {{^ VIRTUAL void b\(\) override NOT_OVERRIDE;}} + + NOT_VIRTUAL void c() OVERRIDE; + // CHECK: {{^ NOT_VIRTUAL void c\(\) OVERRIDE;}} + + VIRTUAL void d() OVERRIDE; + // CHECK: {{^ VIRTUAL void d\(\) OVERRIDE;}} +};