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,133 @@ +//===--- 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); +} + +// Re-lex the tokens to get precise locations to insert 'override' and remove +// 'virtual'. +static SmallVector ParseTokens(CharSourceRange Range, + const SourceManager &Sources, + LangOptions LangOpts) { + std::pair LocInfo = + Sources.getDecomposedLoc(Range.getBegin()); + StringRef File = Sources.getBufferData(LocInfo.first); + const char *TokenBegin = File.data() + LocInfo.second; + Lexer RawLexer(Sources.getLocForStartOfFile(LocInfo.first), LangOpts, + File.begin(), TokenBegin, File.end()); + SmallVector Tokens; + Token Tok; + while (!RawLexer.LexFromRawLexer(Tok)) { + if (Tok.is(tok::semi) || Tok.is(tok::l_brace)) + break; + if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation())) + break; + Tokens.push_back(Tok); + } + return Tokens; +} + +static StringRef GetText(const Token& Tok, const SourceManager &Sources) { + return {Sources.getCharacterData(Tok.getLocation()), Tok.getLength()}; +} + +void UseOverride::check(const MatchFinder::MatchResult &Result) { + const FunctionDecl *Method = Result.Nodes.getStmtAs("method"); + const SourceManager &Sources = *Result.SourceManager; + + Method->dump(); + + 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()), + Sources, Result.Context->getLangOpts()); + + if (!FileRange.isValid()) + return; + + // FIXME: Instead of re-lexing and looking for specific macros such as + // 'ABSTRACT', properly store the location of 'virtual' and '= 0' in each + // FunctionDecl. + SmallVector Tokens = ParseTokens(FileRange, Sources, + Result.Context->getLangOpts()); + + // 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 = Sources.getExpansionLoc(attr->getLocation()); + break; + } + } + } + + if (InsertLoc.isInvalid() && Method->doesThisDeclarationHaveABody()) { + InsertLoc = Method->getBody()->getLocStart(); + } + + if (!InsertLoc.isValid()) { + if (Tokens.size() > 2 && GetText(Tokens.back(), Sources) == "0" && + GetText(Tokens[Tokens.size() - 2], Sources) == "=") { + InsertLoc = Tokens[Tokens.size() - 2].getLocation(); + } else if (GetText(Tokens.back(), Sources) == "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) && + GetText(Tokens[i], Sources) == "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,134 @@ +// 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 + +#define MUST_USE_RESULT __attribute__((warn_unused_result)) + +struct MUST_USE_RESULT MustUseResultObject {}; + +struct Base { + virtual ~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 j() const; + virtual MustUseResultObject k(); + virtual bool l() MUST_USE_RESULT; +}; + +struct SimpleCases : public Base { +public: + virtual ~SimpleCases(); + // CHECK: {{^ ~SimpleCases\(\) override;}} + + 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 j() const; + // CHECK: {{^ void j\(\) const override;}} + virtual MustUseResultObject k(); // Has an implicit attribute. + // CHECK: {{^ MustUseResultObject k\(\) override;}} + virtual bool l() MUST_USE_RESULT; // Has an explicit attribute + // CHECK: {{^ bool l\(\) override MUST_USE_RESULT;}} +}; + +void SimpleCases::i() {} +// CHECK: {{^void SimpleCases::i\(\) {}}} + +struct InlineDefinitions : public Base { +public: + virtual ~InlineDefinitions() {} + // CHECK: {{^ ~InlineDefinitions\(\) override {}}} + + 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 j() const {} + // CHECK: {{^ void j\(\) const override {}}} + virtual MustUseResultObject k() {} // Has an implicit attribute. + // CHECK: {{^ MustUseResultObject k\(\) override {}}} + virtual bool l() MUST_USE_RESULT {} // Has an explicit attribute + // CHECK: {{^ bool l\(\) override MUST_USE_RESULT {}}} +}; + +struct Macros : public Base { + // Tests for 'virtual' and 'override' being defined through macros. Basically + // give up for now. + 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;}} + +#define FUNC(name, return_type) return_type name() + FUNC(void, e); + // CHECK: {{^ FUNC\(void, e\);}} + +#define F virtual void f(); + F + // CHECK: {{^ F}} +}; + +// Tests for templates. +template struct TemplateBase { + virtual void f(T t); +}; + +template struct DerivedFromTemplate : public TemplateBase { + virtual void f(T t); + // CHECK: {{^ void f\(T t\) override;}} +}; +void f() { DerivedFromTemplate().f(2); } + +template +struct UnusedMemberInstantiation : public C { + virtual ~UnusedMemberInstantiation() {} + // CHECK: {{^ ~UnusedMemberInstantiation\(\) override {}}} +}; +struct IntantiateWithoutUse : public UnusedMemberInstantiation {}; + +// The OverrideAttr isn't propagated to specializations in all cases. Make sure +// we don't add "override" a second time. +template +struct MembersOfSpecializations : public Base { + void a() override; + // CHECK: {{^ void a\(\) override;}} +}; +template <> void MembersOfSpecializations<3>::a() {} +void f() { D<3>().a(); };