Index: clang-tidy/google/GoogleTidyModule.cpp =================================================================== --- clang-tidy/google/GoogleTidyModule.cpp +++ clang-tidy/google/GoogleTidyModule.cpp @@ -11,9 +11,9 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "../readability/BracesAroundStatementsCheck.h" -#include "../readability/FunctionSize.h" +#include "../readability/FunctionSizeCheck.h" #include "../readability/NamespaceCommentCheck.h" -#include "../readability/RedundantSmartptrGet.h" +#include "../readability/RedundantSmartptrGetCheck.h" #include "AvoidCStyleCastsCheck.h" #include "ExplicitConstructorCheck.h" #include "ExplicitMakePairCheck.h" @@ -69,7 +69,7 @@ .registerCheck( "google-readability-namespace-comments"); CheckFactories - .registerCheck( + .registerCheck( "google-readability-redundant-smartptr-get"); } Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -2,12 +2,12 @@ add_clang_library(clangTidyReadabilityModule BracesAroundStatementsCheck.cpp - ContainerSizeEmpty.cpp + ContainerSizeEmptyCheck.cpp ElseAfterReturnCheck.cpp - FunctionSize.cpp + FunctionSizeCheck.cpp NamespaceCommentCheck.cpp ReadabilityTidyModule.cpp - RedundantSmartptrGet.cpp + RedundantSmartptrGetCheck.cpp ShrinkToFitCheck.cpp LINK_LIBS Index: clang-tidy/readability/ContainerSizeEmpty.h =================================================================== --- clang-tidy/readability/ContainerSizeEmpty.h +++ /dev/null @@ -1,40 +0,0 @@ -//===--- ContainerSizeEmpty.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_CONTAINERSIZEEMPTY_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINERSIZEEMPTY_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace readability { - -/// \brief Checks whether a call to the \c size() method can be replaced with a -/// call to \c empty(). -/// -/// The emptiness of a container should be checked using the \c empty() method -/// instead of the \c size() method. It is not guaranteed that \c size() is a -/// constant-time function, and it is generally more efficient and also shows -/// clearer intent to use \c empty(). Furthermore some containers may implement -/// the \c empty() method but not implement the \c size() method. Using \c -/// empty() whenever possible makes it easier to switch to another container in -/// the future. -class ContainerSizeEmptyCheck : public ClangTidyCheck { -public: - ContainerSizeEmptyCheck(StringRef Name, ClangTidyContext *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_CONTAINERSIZEEMPTY_H Index: clang-tidy/readability/ContainerSizeEmpty.cpp =================================================================== --- clang-tidy/readability/ContainerSizeEmpty.cpp +++ /dev/null @@ -1,163 +0,0 @@ -//===--- ContainerSizeEmpty.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 "ContainerSizeEmpty.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/Lex/Lexer.h" -#include "llvm/ADT/StringRef.h" -#include "llvm/ADT/StringSet.h" - -using namespace clang::ast_matchers; - -namespace { -bool isContainer(llvm::StringRef ClassName) { - static const llvm::StringSet<> ContainerNames = [] { - llvm::StringSet<> RetVal; - RetVal.insert("std::vector"); - RetVal.insert("std::list"); - RetVal.insert("std::array"); - RetVal.insert("std::deque"); - RetVal.insert("std::forward_list"); - RetVal.insert("std::set"); - RetVal.insert("std::map"); - RetVal.insert("std::multiset"); - RetVal.insert("std::multimap"); - RetVal.insert("std::unordered_set"); - RetVal.insert("std::unordered_map"); - RetVal.insert("std::unordered_multiset"); - RetVal.insert("std::unordered_multimap"); - RetVal.insert("std::stack"); - RetVal.insert("std::queue"); - RetVal.insert("std::priority_queue"); - return RetVal; - }(); - return ContainerNames.find(ClassName) != ContainerNames.end(); -} -} // namespace - -namespace clang { -namespace ast_matchers { -AST_MATCHER(QualType, isBoolType) { return Node->isBooleanType(); } - -AST_MATCHER(NamedDecl, stlContainer) { - return isContainer(Node.getQualifiedNameAsString()); -} -} // namespace ast_matchers -} // namespace clang - -namespace clang { -namespace tidy { -namespace readability { - -ContainerSizeEmptyCheck::ContainerSizeEmptyCheck(StringRef Name, - ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - -void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { - const auto WrongUse = anyOf( - hasParent( - binaryOperator( - anyOf(has(integerLiteral(equals(0))), - allOf(anyOf(hasOperatorName("<"), hasOperatorName(">="), - hasOperatorName(">"), hasOperatorName("<=")), - hasEitherOperand(integerLiteral(equals(1)))))) - .bind("SizeBinaryOp")), - hasParent(implicitCastExpr( - hasImplicitDestinationType(isBoolType()), - anyOf( - hasParent(unaryOperator(hasOperatorName("!")).bind("NegOnSize")), - anything()))), - hasParent(explicitCastExpr(hasDestinationType(isBoolType())))); - - Finder->addMatcher( - memberCallExpr( - on(expr(anyOf(hasType(namedDecl(stlContainer())), - hasType(pointsTo(namedDecl(stlContainer()))), - hasType(references(namedDecl(stlContainer()))))) - .bind("STLObject")), - callee(methodDecl(hasName("size"))), WrongUse).bind("SizeCallExpr"), - this); -} - -void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) { - const auto *MemberCall = - Result.Nodes.getNodeAs("SizeCallExpr"); - const auto *BinaryOp = Result.Nodes.getNodeAs("SizeBinaryOp"); - const auto *E = Result.Nodes.getNodeAs("STLObject"); - FixItHint Hint; - std::string ReplacementText = Lexer::getSourceText( - CharSourceRange::getTokenRange(E->getSourceRange()), - *Result.SourceManager, Result.Context->getLangOpts()); - if (E->getType()->isPointerType()) - ReplacementText += "->empty()"; - else - ReplacementText += ".empty()"; - - if (BinaryOp) { // Determine the correct transformation. - bool Negation = false; - const bool ContainerIsLHS = !llvm::isa(BinaryOp->getLHS()); - const auto OpCode = BinaryOp->getOpcode(); - uint64_t Value = 0; - if (ContainerIsLHS) { - if (const auto *Literal = - llvm::dyn_cast(BinaryOp->getRHS())) - Value = Literal->getValue().getLimitedValue(); - else - return; - } else { - Value = llvm::dyn_cast(BinaryOp->getLHS()) - ->getValue() - .getLimitedValue(); - } - - // Constant that is not handled. - if (Value > 1) - return; - - // Always true, no warnings for that. - if ((OpCode == BinaryOperatorKind::BO_GE && Value == 0 && ContainerIsLHS) || - (OpCode == BinaryOperatorKind::BO_LE && Value == 0 && !ContainerIsLHS)) - return; - - if (OpCode == BinaryOperatorKind::BO_NE && Value == 0) - Negation = true; - if ((OpCode == BinaryOperatorKind::BO_GT || - OpCode == BinaryOperatorKind::BO_GE) && - ContainerIsLHS) - Negation = true; - if ((OpCode == BinaryOperatorKind::BO_LT || - OpCode == BinaryOperatorKind::BO_LE) && - !ContainerIsLHS) - Negation = true; - - if (Negation) - ReplacementText = "!" + ReplacementText; - Hint = FixItHint::CreateReplacement(BinaryOp->getSourceRange(), - ReplacementText); - - } else { - // If there is a conversion above the size call to bool, it is safe to just - // replace size with empty. - if (const auto *UnaryOp = - Result.Nodes.getNodeAs("NegOnSize")) - Hint = FixItHint::CreateReplacement(UnaryOp->getSourceRange(), - ReplacementText); - else - Hint = FixItHint::CreateReplacement(MemberCall->getSourceRange(), - "!" + ReplacementText); - } - diag(MemberCall->getLocStart(), - "The 'empty' method should be used to check for emptiness instead " - "of 'size'.") - << Hint; -} - -} // namespace readability -} // namespace tidy -} // namespace clang Index: clang-tidy/readability/ContainerSizeEmptyCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/ContainerSizeEmptyCheck.h @@ -0,0 +1,40 @@ +//===--- ContainerSizeEmptyCheck.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_CONTAINER_SIZE_EMPTY_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONTAINER_SIZE_EMPTY_CHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// \brief Checks whether a call to the \c size() method can be replaced with a +/// call to \c empty(). +/// +/// The emptiness of a container should be checked using the \c empty() method +/// instead of the \c size() method. It is not guaranteed that \c size() is a +/// constant-time function, and it is generally more efficient and also shows +/// clearer intent to use \c empty(). Furthermore some containers may implement +/// the \c empty() method but not implement the \c size() method. Using \c +/// empty() whenever possible makes it easier to switch to another container in +/// the future. +class ContainerSizeEmptyCheck : public ClangTidyCheck { +public: + ContainerSizeEmptyCheck(StringRef Name, ClangTidyContext *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_CONTAINER_SIZE_EMPTY_CHECK_H Index: clang-tidy/readability/ContainerSizeEmptyCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/ContainerSizeEmptyCheck.cpp @@ -0,0 +1,163 @@ +//===--- ContainerSizeEmptyCheck.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 "ContainerSizeEmptyCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Lex/Lexer.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/ADT/StringSet.h" + +using namespace clang::ast_matchers; + +namespace { +bool isContainer(llvm::StringRef ClassName) { + static const llvm::StringSet<> ContainerNames = [] { + llvm::StringSet<> RetVal; + RetVal.insert("std::vector"); + RetVal.insert("std::list"); + RetVal.insert("std::array"); + RetVal.insert("std::deque"); + RetVal.insert("std::forward_list"); + RetVal.insert("std::set"); + RetVal.insert("std::map"); + RetVal.insert("std::multiset"); + RetVal.insert("std::multimap"); + RetVal.insert("std::unordered_set"); + RetVal.insert("std::unordered_map"); + RetVal.insert("std::unordered_multiset"); + RetVal.insert("std::unordered_multimap"); + RetVal.insert("std::stack"); + RetVal.insert("std::queue"); + RetVal.insert("std::priority_queue"); + return RetVal; + }(); + return ContainerNames.find(ClassName) != ContainerNames.end(); +} +} // namespace + +namespace clang { +namespace ast_matchers { +AST_MATCHER(QualType, isBoolType) { return Node->isBooleanType(); } + +AST_MATCHER(NamedDecl, stlContainer) { + return isContainer(Node.getQualifiedNameAsString()); +} +} // namespace ast_matchers +} // namespace clang + +namespace clang { +namespace tidy { +namespace readability { + +ContainerSizeEmptyCheck::ContainerSizeEmptyCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + +void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) { + const auto WrongUse = anyOf( + hasParent( + binaryOperator( + anyOf(has(integerLiteral(equals(0))), + allOf(anyOf(hasOperatorName("<"), hasOperatorName(">="), + hasOperatorName(">"), hasOperatorName("<=")), + hasEitherOperand(integerLiteral(equals(1)))))) + .bind("SizeBinaryOp")), + hasParent(implicitCastExpr( + hasImplicitDestinationType(isBoolType()), + anyOf( + hasParent(unaryOperator(hasOperatorName("!")).bind("NegOnSize")), + anything()))), + hasParent(explicitCastExpr(hasDestinationType(isBoolType())))); + + Finder->addMatcher( + memberCallExpr( + on(expr(anyOf(hasType(namedDecl(stlContainer())), + hasType(pointsTo(namedDecl(stlContainer()))), + hasType(references(namedDecl(stlContainer()))))) + .bind("STLObject")), + callee(methodDecl(hasName("size"))), WrongUse).bind("SizeCallExpr"), + this); +} + +void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MemberCall = + Result.Nodes.getNodeAs("SizeCallExpr"); + const auto *BinaryOp = Result.Nodes.getNodeAs("SizeBinaryOp"); + const auto *E = Result.Nodes.getNodeAs("STLObject"); + FixItHint Hint; + std::string ReplacementText = Lexer::getSourceText( + CharSourceRange::getTokenRange(E->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + if (E->getType()->isPointerType()) + ReplacementText += "->empty()"; + else + ReplacementText += ".empty()"; + + if (BinaryOp) { // Determine the correct transformation. + bool Negation = false; + const bool ContainerIsLHS = !llvm::isa(BinaryOp->getLHS()); + const auto OpCode = BinaryOp->getOpcode(); + uint64_t Value = 0; + if (ContainerIsLHS) { + if (const auto *Literal = + llvm::dyn_cast(BinaryOp->getRHS())) + Value = Literal->getValue().getLimitedValue(); + else + return; + } else { + Value = llvm::dyn_cast(BinaryOp->getLHS()) + ->getValue() + .getLimitedValue(); + } + + // Constant that is not handled. + if (Value > 1) + return; + + // Always true, no warnings for that. + if ((OpCode == BinaryOperatorKind::BO_GE && Value == 0 && ContainerIsLHS) || + (OpCode == BinaryOperatorKind::BO_LE && Value == 0 && !ContainerIsLHS)) + return; + + if (OpCode == BinaryOperatorKind::BO_NE && Value == 0) + Negation = true; + if ((OpCode == BinaryOperatorKind::BO_GT || + OpCode == BinaryOperatorKind::BO_GE) && + ContainerIsLHS) + Negation = true; + if ((OpCode == BinaryOperatorKind::BO_LT || + OpCode == BinaryOperatorKind::BO_LE) && + !ContainerIsLHS) + Negation = true; + + if (Negation) + ReplacementText = "!" + ReplacementText; + Hint = FixItHint::CreateReplacement(BinaryOp->getSourceRange(), + ReplacementText); + + } else { + // If there is a conversion above the size call to bool, it is safe to just + // replace size with empty. + if (const auto *UnaryOp = + Result.Nodes.getNodeAs("NegOnSize")) + Hint = FixItHint::CreateReplacement(UnaryOp->getSourceRange(), + ReplacementText); + else + Hint = FixItHint::CreateReplacement(MemberCall->getSourceRange(), + "!" + ReplacementText); + } + diag(MemberCall->getLocStart(), + "The 'empty' method should be used to check for emptiness instead " + "of 'size'.") + << Hint; +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/FunctionSize.h =================================================================== --- clang-tidy/readability/FunctionSize.h +++ /dev/null @@ -1,48 +0,0 @@ -//===--- FunctionSize.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_FUNCTIONSIZE_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTIONSIZE_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace readability { - -/// \brief Checks for large functions based on various metrics. -class FunctionSizeCheck : public ClangTidyCheck { -public: - FunctionSizeCheck(StringRef Name, ClangTidyContext *Context); - - void storeOptions(ClangTidyOptions::OptionMap &Opts) override; - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - void onEndOfTranslationUnit() override; - -private: - struct FunctionInfo { - FunctionInfo() : Lines(0), Statements(0), Branches(0) {} - unsigned Lines; - unsigned Statements; - unsigned Branches; - }; - - const unsigned LineThreshold; - const unsigned StatementThreshold; - const unsigned BranchThreshold; - - llvm::DenseMap FunctionInfos; -}; - -} // namespace readability -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTIONSIZE_H Index: clang-tidy/readability/FunctionSize.cpp =================================================================== --- clang-tidy/readability/FunctionSize.cpp +++ /dev/null @@ -1,106 +0,0 @@ -//===--- FunctionSize.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 "FunctionSize.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace readability { - -FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - LineThreshold(Options.get("LineThreshold", -1U)), - StatementThreshold(Options.get("StatementThreshold", 800U)), - BranchThreshold(Options.get("BranchThreshold", -1U)) {} - -void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "LineThreshold", LineThreshold); - Options.store(Opts, "StatementThreshold", StatementThreshold); - Options.store(Opts, "BranchThreshold", BranchThreshold); -} - -void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - functionDecl( - unless(isInstantiated()), - forEachDescendant( - stmt(unless(compoundStmt()), - hasParent(stmt(anyOf(compoundStmt(), ifStmt(), - anyOf(whileStmt(), doStmt(), - forRangeStmt(), forStmt()))))) - .bind("stmt"))).bind("func"), - this); -} - -void FunctionSizeCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Func = Result.Nodes.getNodeAs("func"); - - FunctionInfo &FI = FunctionInfos[Func]; - - // Count the lines including whitespace and comments. Really simple. - if (!FI.Lines) { - if (const Stmt *Body = Func->getBody()) { - SourceManager *SM = Result.SourceManager; - if (SM->isWrittenInSameFile(Body->getLocStart(), Body->getLocEnd())) { - FI.Lines = SM->getSpellingLineNumber(Body->getLocEnd()) - - SM->getSpellingLineNumber(Body->getLocStart()); - } - } - } - - const auto *Statement = Result.Nodes.getNodeAs("stmt"); - ++FI.Statements; - - // TODO: switch cases, gotos - if (isa(Statement) || isa(Statement) || - isa(Statement) || isa(Statement) || - isa(Statement) || isa(Statement)) - ++FI.Branches; -} - -void FunctionSizeCheck::onEndOfTranslationUnit() { - // If we're above the limit emit a warning. - for (const auto &P : FunctionInfos) { - const FunctionInfo &FI = P.second; - if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold || - FI.Branches > BranchThreshold) { - diag(P.first->getLocation(), - "function '%0' exceeds recommended size/complexity thresholds") - << P.first->getNameAsString(); - } - - if (FI.Lines > LineThreshold) { - diag(P.first->getLocation(), - "%0 lines including whitespace and comments (threshold %1)", - DiagnosticIDs::Note) - << FI.Lines << LineThreshold; - } - - if (FI.Statements > StatementThreshold) { - diag(P.first->getLocation(), "%0 statements (threshold %1)", - DiagnosticIDs::Note) - << FI.Statements << StatementThreshold; - } - - if (FI.Branches > BranchThreshold) { - diag(P.first->getLocation(), "%0 branches (threshold %1)", - DiagnosticIDs::Note) - << FI.Branches << BranchThreshold; - } - } - - FunctionInfos.clear(); -} - -} // namespace readability -} // namespace tidy -} // namespace clang Index: clang-tidy/readability/FunctionSizeCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/FunctionSizeCheck.h @@ -0,0 +1,48 @@ +//===--- FunctionSizeCheck.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_FUNCTION_SIZE_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTION_SIZE_CHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// \brief Checks for large functions based on various metrics. +class FunctionSizeCheck : public ClangTidyCheck { +public: + FunctionSizeCheck(StringRef Name, ClangTidyContext *Context); + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void onEndOfTranslationUnit() override; + +private: + struct FunctionInfo { + FunctionInfo() : Lines(0), Statements(0), Branches(0) {} + unsigned Lines; + unsigned Statements; + unsigned Branches; + }; + + const unsigned LineThreshold; + const unsigned StatementThreshold; + const unsigned BranchThreshold; + + llvm::DenseMap FunctionInfos; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_FUNCTION_SIZE_CHECK_H Index: clang-tidy/readability/FunctionSizeCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/FunctionSizeCheck.cpp @@ -0,0 +1,106 @@ +//===--- FunctionSize.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 "FunctionSizeCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + LineThreshold(Options.get("LineThreshold", -1U)), + StatementThreshold(Options.get("StatementThreshold", 800U)), + BranchThreshold(Options.get("BranchThreshold", -1U)) {} + +void FunctionSizeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "LineThreshold", LineThreshold); + Options.store(Opts, "StatementThreshold", StatementThreshold); + Options.store(Opts, "BranchThreshold", BranchThreshold); +} + +void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + functionDecl( + unless(isInstantiated()), + forEachDescendant( + stmt(unless(compoundStmt()), + hasParent(stmt(anyOf(compoundStmt(), ifStmt(), + anyOf(whileStmt(), doStmt(), + forRangeStmt(), forStmt()))))) + .bind("stmt"))).bind("func"), + this); +} + +void FunctionSizeCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Func = Result.Nodes.getNodeAs("func"); + + FunctionInfo &FI = FunctionInfos[Func]; + + // Count the lines including whitespace and comments. Really simple. + if (!FI.Lines) { + if (const Stmt *Body = Func->getBody()) { + SourceManager *SM = Result.SourceManager; + if (SM->isWrittenInSameFile(Body->getLocStart(), Body->getLocEnd())) { + FI.Lines = SM->getSpellingLineNumber(Body->getLocEnd()) - + SM->getSpellingLineNumber(Body->getLocStart()); + } + } + } + + const auto *Statement = Result.Nodes.getNodeAs("stmt"); + ++FI.Statements; + + // TODO: switch cases, gotos + if (isa(Statement) || isa(Statement) || + isa(Statement) || isa(Statement) || + isa(Statement) || isa(Statement)) + ++FI.Branches; +} + +void FunctionSizeCheck::onEndOfTranslationUnit() { + // If we're above the limit emit a warning. + for (const auto &P : FunctionInfos) { + const FunctionInfo &FI = P.second; + if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold || + FI.Branches > BranchThreshold) { + diag(P.first->getLocation(), + "function '%0' exceeds recommended size/complexity thresholds") + << P.first->getNameAsString(); + } + + if (FI.Lines > LineThreshold) { + diag(P.first->getLocation(), + "%0 lines including whitespace and comments (threshold %1)", + DiagnosticIDs::Note) + << FI.Lines << LineThreshold; + } + + if (FI.Statements > StatementThreshold) { + diag(P.first->getLocation(), "%0 statements (threshold %1)", + DiagnosticIDs::Note) + << FI.Statements << StatementThreshold; + } + + if (FI.Branches > BranchThreshold) { + diag(P.first->getLocation(), "%0 branches (threshold %1)", + DiagnosticIDs::Note) + << FI.Branches << BranchThreshold; + } + } + + FunctionInfos.clear(); +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -11,10 +11,10 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "BracesAroundStatementsCheck.h" -#include "ContainerSizeEmpty.h" +#include "ContainerSizeEmptyCheck.h" #include "ElseAfterReturnCheck.h" -#include "FunctionSize.h" -#include "RedundantSmartptrGet.h" +#include "FunctionSizeCheck.h" +#include "RedundantSmartptrGetCheck.h" #include "ShrinkToFitCheck.h" namespace clang { @@ -32,7 +32,7 @@ "readability-else-after-return"); CheckFactories.registerCheck( "readability-function-size"); - CheckFactories.registerCheck( + CheckFactories.registerCheck( "readability-redundant-smartptr-get"); CheckFactories.registerCheck( "readability-shrink-to-fit"); Index: clang-tidy/readability/RedundantSmartptrGet.h =================================================================== --- clang-tidy/readability/RedundantSmartptrGet.h +++ /dev/null @@ -1,38 +0,0 @@ -//===--- RedundantSmartptrGet.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_REDUNDANTSMARTPTRGET_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTSMARTPTRGET_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace readability { - -/// \brief Find and remove redundant calls to smart pointer's .get() method. -/// -/// Examples: -/// ptr.get()->Foo() ==> ptr->Foo() -/// *ptr.get() ==> *ptr -/// *ptr->get() ==> **ptr -class RedundantSmartptrGet : public ClangTidyCheck { -public: - RedundantSmartptrGet(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_REDUNDANTSMARTPTRGET_H - Index: clang-tidy/readability/RedundantSmartptrGet.cpp =================================================================== --- clang-tidy/readability/RedundantSmartptrGet.cpp +++ /dev/null @@ -1,123 +0,0 @@ -//===--- RedundantSmartptrGet.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 "RedundantSmartptrGet.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Lex/Lexer.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace readability { - -namespace { -internal::Matcher callToGet(internal::Matcher OnClass) { - return memberCallExpr( - on(expr(anyOf(hasType(OnClass), - hasType(qualType(pointsTo(decl(OnClass).bind( - "ptr_to_ptr")))))).bind("smart_pointer")), - unless(callee(memberExpr(hasObjectExpression(thisExpr())))), - callee(methodDecl(hasName("get")))).bind("redundant_get"); -} - -void registerMatchersForGetArrowStart(MatchFinder *Finder, - MatchFinder::MatchCallback *Callback) { - const auto QuacksLikeASmartptr = recordDecl( - recordDecl().bind("duck_typing"), - has(methodDecl(hasName("operator->"), - returns(qualType(pointsTo(type().bind("op->Type")))))), - has(methodDecl(hasName("operator*"), - returns(qualType(references(type().bind("op*Type")))))), - has(methodDecl(hasName("get"), - returns(qualType(pointsTo(type().bind("getType"))))))); - - // Catch 'ptr.get()->Foo()' - Finder->addMatcher(memberExpr(expr().bind("memberExpr"), isArrow(), - hasObjectExpression(ignoringImpCasts( - callToGet(QuacksLikeASmartptr)))), - Callback); - - // Catch '*ptr.get()' or '*ptr->get()' - Finder->addMatcher( - unaryOperator(hasOperatorName("*"), - hasUnaryOperand(callToGet(QuacksLikeASmartptr))), - Callback); -} - -void registerMatchersForGetEquals(MatchFinder *Finder, - MatchFinder::MatchCallback *Callback) { - // This one is harder to do with duck typing. - // The operator==/!= that we are looking for might be member or non-member, - // might be on global namespace or found by ADL, might be a template, etc. - // For now, lets keep a list of known standard types. - - const auto IsAKnownSmartptr = recordDecl( - anyOf(hasName("::std::unique_ptr"), hasName("::std::shared_ptr"))); - - // Matches against nullptr. - Finder->addMatcher( - binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")), - hasEitherOperand(ignoringImpCasts(nullPtrLiteralExpr())), - hasEitherOperand(callToGet(IsAKnownSmartptr))), - Callback); - // TODO: Catch ptr.get() == other_ptr.get() -} - - -} // namespace - -void RedundantSmartptrGet::registerMatchers(MatchFinder *Finder) { - registerMatchersForGetArrowStart(Finder, this); - registerMatchersForGetEquals(Finder, this); -} - -namespace { -bool allReturnTypesMatch(const MatchFinder::MatchResult &Result) { - if (Result.Nodes.getNodeAs("duck_typing") == nullptr) - return true; - // Verify that the types match. - // We can't do this on the matcher because the type nodes can be different, - // even though they represent the same type. This difference comes from how - // the type is referenced (eg. through a typedef, a type trait, etc). - const Type *OpArrowType = - Result.Nodes.getNodeAs("op->Type")->getUnqualifiedDesugaredType(); - const Type *OpStarType = - Result.Nodes.getNodeAs("op*Type")->getUnqualifiedDesugaredType(); - const Type *GetType = - Result.Nodes.getNodeAs("getType")->getUnqualifiedDesugaredType(); - return OpArrowType == OpStarType && OpArrowType == GetType; -} -} // namespace - -void RedundantSmartptrGet::check(const MatchFinder::MatchResult &Result) { - if (!allReturnTypesMatch(Result)) return; - - bool IsPtrToPtr = Result.Nodes.getNodeAs("ptr_to_ptr") != nullptr; - bool IsMemberExpr = Result.Nodes.getNodeAs("memberExpr") != nullptr; - const Expr *GetCall = Result.Nodes.getNodeAs("redundant_get"); - const Expr *Smartptr = Result.Nodes.getNodeAs("smart_pointer"); - - if (IsPtrToPtr && IsMemberExpr) { - // Ignore this case (eg. Foo->get()->DoSomething()); - return; - } - - StringRef SmartptrText = Lexer::getSourceText( - CharSourceRange::getTokenRange(Smartptr->getSourceRange()), - *Result.SourceManager, Result.Context->getLangOpts()); - // Replace foo->get() with *foo, and foo.get() with foo. - std::string Replacement = Twine(IsPtrToPtr ? "*" : "", SmartptrText).str(); - diag(GetCall->getLocStart(), "Redundant get() call on smart pointer.") - << FixItHint::CreateReplacement(GetCall->getSourceRange(), Replacement); -} - -} // namespace readability -} // namespace tidy -} // namespace clang Index: clang-tidy/readability/RedundantSmartptrGetCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantSmartptrGetCheck.h @@ -0,0 +1,37 @@ +//===--- RedundantSmartptrGetCheck.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_SMART_PTR_GET_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_SMART_PTR_GET_CHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// \brief Find and remove redundant calls to smart pointer's .get() method. +/// +/// Examples: +/// ptr.get()->Foo() ==> ptr->Foo() +/// *ptr.get() ==> *ptr +/// *ptr->get() ==> **ptr +class RedundantSmartptrGetCheck : public ClangTidyCheck { +public: + RedundantSmartptrGetCheck(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_SMART_PTR_GET_CHECK_H Index: clang-tidy/readability/RedundantSmartptrGetCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/RedundantSmartptrGetCheck.cpp @@ -0,0 +1,123 @@ +//===--- RedundantSmartptrGetCheck.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 "RedundantSmartptrGetCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +namespace { +internal::Matcher callToGet(internal::Matcher OnClass) { + return memberCallExpr( + on(expr(anyOf(hasType(OnClass), + hasType(qualType(pointsTo(decl(OnClass).bind( + "ptr_to_ptr")))))).bind("smart_pointer")), + unless(callee(memberExpr(hasObjectExpression(thisExpr())))), + callee(methodDecl(hasName("get")))).bind("redundant_get"); +} + +void registerMatchersForGetArrowStart(MatchFinder *Finder, + MatchFinder::MatchCallback *Callback) { + const auto QuacksLikeASmartptr = recordDecl( + recordDecl().bind("duck_typing"), + has(methodDecl(hasName("operator->"), + returns(qualType(pointsTo(type().bind("op->Type")))))), + has(methodDecl(hasName("operator*"), + returns(qualType(references(type().bind("op*Type")))))), + has(methodDecl(hasName("get"), + returns(qualType(pointsTo(type().bind("getType"))))))); + + // Catch 'ptr.get()->Foo()' + Finder->addMatcher(memberExpr(expr().bind("memberExpr"), isArrow(), + hasObjectExpression(ignoringImpCasts( + callToGet(QuacksLikeASmartptr)))), + Callback); + + // Catch '*ptr.get()' or '*ptr->get()' + Finder->addMatcher( + unaryOperator(hasOperatorName("*"), + hasUnaryOperand(callToGet(QuacksLikeASmartptr))), + Callback); +} + +void registerMatchersForGetEquals(MatchFinder *Finder, + MatchFinder::MatchCallback *Callback) { + // This one is harder to do with duck typing. + // The operator==/!= that we are looking for might be member or non-member, + // might be on global namespace or found by ADL, might be a template, etc. + // For now, lets keep a list of known standard types. + + const auto IsAKnownSmartptr = recordDecl( + anyOf(hasName("::std::unique_ptr"), hasName("::std::shared_ptr"))); + + // Matches against nullptr. + Finder->addMatcher( + binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")), + hasEitherOperand(ignoringImpCasts(nullPtrLiteralExpr())), + hasEitherOperand(callToGet(IsAKnownSmartptr))), + Callback); + // TODO: Catch ptr.get() == other_ptr.get() +} + + +} // namespace + +void RedundantSmartptrGetCheck::registerMatchers(MatchFinder *Finder) { + registerMatchersForGetArrowStart(Finder, this); + registerMatchersForGetEquals(Finder, this); +} + +namespace { +bool allReturnTypesMatch(const MatchFinder::MatchResult &Result) { + if (Result.Nodes.getNodeAs("duck_typing") == nullptr) + return true; + // Verify that the types match. + // We can't do this on the matcher because the type nodes can be different, + // even though they represent the same type. This difference comes from how + // the type is referenced (eg. through a typedef, a type trait, etc). + const Type *OpArrowType = + Result.Nodes.getNodeAs("op->Type")->getUnqualifiedDesugaredType(); + const Type *OpStarType = + Result.Nodes.getNodeAs("op*Type")->getUnqualifiedDesugaredType(); + const Type *GetType = + Result.Nodes.getNodeAs("getType")->getUnqualifiedDesugaredType(); + return OpArrowType == OpStarType && OpArrowType == GetType; +} +} // namespace + +void RedundantSmartptrGetCheck::check(const MatchFinder::MatchResult &Result) { + if (!allReturnTypesMatch(Result)) return; + + bool IsPtrToPtr = Result.Nodes.getNodeAs("ptr_to_ptr") != nullptr; + bool IsMemberExpr = Result.Nodes.getNodeAs("memberExpr") != nullptr; + const Expr *GetCall = Result.Nodes.getNodeAs("redundant_get"); + const Expr *Smartptr = Result.Nodes.getNodeAs("smart_pointer"); + + if (IsPtrToPtr && IsMemberExpr) { + // Ignore this case (eg. Foo->get()->DoSomething()); + return; + } + + StringRef SmartptrText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Smartptr->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + // Replace foo->get() with *foo, and foo.get() with foo. + std::string Replacement = Twine(IsPtrToPtr ? "*" : "", SmartptrText).str(); + diag(GetCall->getLocStart(), "Redundant get() call on smart pointer.") + << FixItHint::CreateReplacement(GetCall->getSourceRange(), Replacement); +} + +} // namespace readability +} // namespace tidy +} // namespace clang