Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -13,6 +13,7 @@ NamespaceCommentCheck.cpp ReadabilityTidyModule.cpp RedundantControlFlowCheck.cpp + RedundantInlineCheck.cpp RedundantStringCStrCheck.cpp RedundantSmartptrGetCheck.cpp RedundantStringInitCheck.cpp Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -20,6 +20,7 @@ #include "InconsistentDeclarationParameterNameCheck.h" #include "NamedParameterCheck.h" #include "RedundantControlFlowCheck.h" +#include "RedundantInlineCheck.h" #include "RedundantSmartptrGetCheck.h" #include "RedundantStringCStrCheck.h" #include "RedundantStringInitCheck.h" @@ -50,6 +51,8 @@ "readability-implicit-bool-cast"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); + CheckFactories.registerCheck( + "readability-redundant-inline"); CheckFactories.registerCheck( "readability-static-definition-in-anonymous-namespace"); CheckFactories.registerCheck( Index: clang-tidy/readability/RedundantInlineCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantInlineCheck.h @@ -0,0 +1,35 @@ +//===--- RedundantInlineCheck.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_READABILITY_REDUNDANT_INLINE_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_INLINE_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Flags redundant 'inline' when used on a method with inline body or on a constexpr function. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-inline.html +class RedundantInlineCheck : public ClangTidyCheck { +public: + RedundantInlineCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_INLINE_H Index: clang-tidy/readability/RedundantInlineCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantInlineCheck.cpp @@ -0,0 +1,108 @@ +//===--- RedundantInlineCheck.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 "RedundantInlineCheck.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 { +namespace readability { + +AST_MATCHER(CXXMethodDecl, hasInlineBody) { return Node.hasInlineBody(); } + +AST_MATCHER(FriendDecl, isInlineAndHasBody) { + NamedDecl *D = Node.getFriendDecl(); + if (!D) + return false; + auto *F = dyn_cast(D); + if (!F) + return false; + return F->isThisDeclarationADefinition() && F->isInlineSpecified(); +} + +void RedundantInlineCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) + return; + Finder->addMatcher(functionDecl(isInline(), isConstexpr()).bind("func"), + this); + Finder->addMatcher( + cxxMethodDecl(isInline(), unless(isConstexpr()), hasInlineBody()) + .bind("method"), + this); + Finder->addMatcher(friendDecl(isInlineAndHasBody()).bind("friend"), this); +} + +// Re-lex the tokens to get precise location of 'inline' +static Token InlineTok(CharSourceRange Range, const MatchFinder::MatchResult &Result) { + const SourceManager &Sources = *Result.SourceManager; + 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), + Result.Context->getLangOpts(), File.begin(), TokenBegin, + File.end()); + Token Tok; + while (!RawLexer.LexFromRawLexer(Tok)) { + if (Tok.is(tok::raw_identifier)) { + IdentifierInfo &Info = Result.Context->Idents.get(StringRef( + Sources.getCharacterData(Tok.getLocation()), Tok.getLength())); + Tok.setIdentifierInfo(&Info); + Tok.setKind(Info.getTokenID()); + } + if (Tok.is(tok::kw_inline)) + return Tok; + if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation())) + break; + } + llvm_unreachable("InlineTok() did not encounter the 'inline' token"); +} + +void RedundantInlineCheck::check(const MatchFinder::MatchResult &Result) { + const char *Msg; + SourceRange Range; + SourceLocation Loc; + if (const auto *Matched = Result.Nodes.getNodeAs("method")) { + Msg = "'inline' is redundant because method body is defined inside class"; + Range = SourceRange(Matched->getSourceRange().getBegin(), Matched->getLocation()); + Loc = Matched->getLocation(); + } else if (const auto *Matched = + Result.Nodes.getNodeAs("func")) { + Msg = "'inline' is redundant because 'constexpr' implies 'inline'"; + Range = SourceRange(Matched->getSourceRange().getBegin(), Matched->getLocation()); + Loc = Matched->getLocation(); + } else if (const auto *Matched = + Result.Nodes.getNodeAs("friend")) { + Msg = "'inline' is redundant because function body is defined inside class"; + Range = SourceRange(Matched->getSourceRange().getBegin(), Matched->getLocation()); + Loc = Matched->getLocation(); + } else { + return; + } + + CharSourceRange FileRange = Lexer::makeFileCharRange( + CharSourceRange::getTokenRange(Range), *Result.SourceManager, + Result.Context->getLangOpts()); + + if (!FileRange.isValid()) + return; + + // FIXME: Instead of re-lexing, properly store the location of 'inline' in + // each FunctionDecl. + Token Tok = InlineTok(FileRange, Result); + diag(Loc, Msg) << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(Tok.getLocation(), Tok.getLocation())); +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -160,6 +160,12 @@ Looks for procedures (functions returning no value) with ``return`` statements at the end of the function. Such `return` statements are redundant. +- New `readability-redundant-inline + `_ check + + Looks for redundant ``inline`` specifiers which are implied by defining a body within a class definition + or by ``constexpr``. + - New `readability-redundant-string-init `_ check Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -105,6 +105,7 @@ readability-inconsistent-declaration-parameter-name readability-named-parameter readability-redundant-control-flow + readability-redundant-inline readability-redundant-smartptr-get readability-redundant-string-cstr readability-redundant-string-init Index: docs/clang-tidy/checks/readability-redundant-inline.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-redundant-inline.rst @@ -0,0 +1,16 @@ +.. title:: clang-tidy - readability-redundant-inline + +readability-redundant-inline +============================ + +This check flags redundant ``inline`` specifiers. +It flags ``inline`` on member functions defined inside a class definition like +.. code-block:: c++ + + struct S { + inline int f() { + return 0; + } + }; + +and ``inline`` specifiers on functions that are also declared ``constexpr``. Index: test/clang-tidy/readability-redundant-inline.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-redundant-inline.cpp @@ -0,0 +1,72 @@ +// RUN: %check_clang_tidy %s readability-redundant-inline %t + +struct S { + inline int f1() { +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'inline' is redundant because method body is defined inside class [readability-redundant-inline] +// CHECK-FIXES: {{^}} int f1() + return 0; + } + inline int f2(); // OK + + inline constexpr int f3() { +// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'inline' is redundant because 'constexpr' implies 'inline' [readability-redundant-inline] +// CHECK-FIXES: {{^}} constexpr int f3() + return 0; + } + static inline constexpr int f4() { +// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 'inline' is redundant because 'constexpr' implies 'inline' +// CHECK-FIXES: {{^}} static constexpr int f4() + return 0; + } + + static inline int f5(); + + static inline int f6() { +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'inline' is redundant because method body is defined inside class +// CHECK-FIXES: {{^}} static int f6() + return 0; + } + + inline friend int f7() { +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'inline' is redundant because function body is defined inside class [readability-redundant-inline] +// CHECK-FIXES: {{^}} friend int f7() + return 0; + } + + inline friend int f8(); // OK + + template struct T{}; + struct T inline f9() { return {}; } +// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'inline' is redundant because method body is defined inside class +// CHECK-FIXES: {{^}} struct T f9() { return {}; } +}; + +class S2 { + inline int f1() { +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'inline' is redundant because method body is defined inside class +// CHECK-FIXES: {{^}} int f1() + return 0; + } +}; + +inline int S::f2() { // OK + return 0; +} + +inline int S::f5() { // OK + return 0; +} + +inline int f3() { // OK + return 0; +} + +inline constexpr int f4() { +// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 'inline' is redundant because 'constexpr' implies 'inline' +// CHECK-FIXES: {{^}}constexpr int f4() + return 1; +} + +constexpr int f5() { // OK + return 1; +}