Index: clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt @@ -3,7 +3,7 @@ add_clang_library(clangTidyPerformanceModule FasterStringFindCheck.cpp ForRangeCopyCheck.cpp - ImplicitCastInLoopCheck.cpp + ImplicitConversionInLoopCheck.cpp InefficientStringConcatenationCheck.cpp InefficientVectorOperationCheck.cpp PerformanceTidyModule.cpp Index: clang-tools-extra/trunk/clang-tidy/performance/ImplicitCastInLoopCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ImplicitCastInLoopCheck.h +++ clang-tools-extra/trunk/clang-tidy/performance/ImplicitCastInLoopCheck.h @@ -1,38 +0,0 @@ -//===--- ImplicitCastInLoopCheck.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_PERFORMANCE_IMPLICIT_CAST_IN_LOOP_CHECK_H_ -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_IMPLICIT_CAST_IN_LOOP_CHECK_H_ - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace performance { - -// Checks that in a for range loop, if the provided type is a reference, then -// the underlying type is the one returned by the iterator (i.e. that there -// isn't any implicit conversion). -class ImplicitCastInLoopCheck : public ClangTidyCheck { -public: - ImplicitCastInLoopCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - -private: - void ReportAndFix(const ASTContext *Context, const VarDecl *VD, - const CXXOperatorCallExpr *OperatorCall); -}; - -} // namespace performance -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_IMPLICIT_CAST_IN_LOOP_CHECK_H_ Index: clang-tools-extra/trunk/clang-tidy/performance/ImplicitCastInLoopCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ImplicitCastInLoopCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/performance/ImplicitCastInLoopCheck.cpp @@ -1,97 +0,0 @@ -//===--- ImplicitCastInLoopCheck.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 "ImplicitCastInLoopCheck.h" - -#include "clang/AST/ASTContext.h" -#include "clang/AST/Decl.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/Lex/Lexer.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace performance { - -namespace { -// Checks if the stmt is a ImplicitCastExpr with a CastKind that is not a NoOp. -// The subtelty is that in some cases (user defined conversions), we can -// get to ImplicitCastExpr inside each other, with the outer one a NoOp. In this -// case we skip the first cast expr. -bool IsNonTrivialImplicitCast(const Stmt *ST) { - if (const auto *ICE = dyn_cast(ST)) { - return (ICE->getCastKind() != CK_NoOp) || - IsNonTrivialImplicitCast(ICE->getSubExpr()); - } - return false; -} -} // namespace - -void ImplicitCastInLoopCheck::registerMatchers(MatchFinder *Finder) { - // We look for const ref loop variables that (optionally inside an - // ExprWithCleanup) materialize a temporary, and contain a implicit cast. The - // check on the implicit cast is done in check() because we can't access - // implicit cast subnode via matchers: has() skips casts and materialize! - // We also bind on the call to operator* to get the proper type in the - // diagnostic message. - // Note that when the implicit cast is done through a user defined cast - // operator, the node is a CXXMemberCallExpr, not a CXXOperatorCallExpr, so - // it should not get caught by the cxxOperatorCallExpr() matcher. - Finder->addMatcher( - cxxForRangeStmt(hasLoopVariable( - varDecl(hasType(qualType(references(qualType(isConstQualified())))), - hasInitializer(expr(hasDescendant(cxxOperatorCallExpr().bind( - "operator-call"))) - .bind("init"))) - .bind("faulty-var"))), - this); -} - -void ImplicitCastInLoopCheck::check(const MatchFinder::MatchResult &Result) { - const auto *VD = Result.Nodes.getNodeAs("faulty-var"); - const auto *Init = Result.Nodes.getNodeAs("init"); - const auto *OperatorCall = - Result.Nodes.getNodeAs("operator-call"); - - if (const auto *Cleanup = dyn_cast(Init)) - Init = Cleanup->getSubExpr(); - - const auto *Materialized = dyn_cast(Init); - if (!Materialized) - return; - - // We ignore NoOp casts. Those are generated if the * operator on the - // iterator returns a value instead of a reference, and the loop variable - // is a reference. This situation is fine (it probably produces the same - // code at the end). - if (IsNonTrivialImplicitCast(Materialized->getTemporary())) - ReportAndFix(Result.Context, VD, OperatorCall); -} - -void ImplicitCastInLoopCheck::ReportAndFix( - const ASTContext *Context, const VarDecl *VD, - const CXXOperatorCallExpr *OperatorCall) { - // We only match on const ref, so we should print a const ref version of the - // type. - QualType ConstType = OperatorCall->getType().withConst(); - QualType ConstRefType = Context->getLValueReferenceType(ConstType); - const char Message[] = - "the type of the loop variable %0 is different from the one returned " - "by the iterator and generates an implicit cast; you can either " - "change the type to the correct one (%1 but 'const auto&' is always a " - "valid option) or remove the reference to make it explicit that you are " - "creating a new value"; - diag(VD->getLocStart(), Message) << VD << ConstRefType; -} - -} // namespace performance -} // namespace tidy -} // namespace clang Index: clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.h +++ clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.h @@ -0,0 +1,38 @@ +//===--- ImplicitConversionInLoopCheck.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_PERFORMANCE_IMPLICIT_CONVERSION_IN_LOOP_CHECK_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_IMPLICIT_CONVERSION_IN_LOOP_CHECK_H_ + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace performance { + +// Checks that in a for range loop, if the provided type is a reference, then +// the underlying type is the one returned by the iterator (i.e. that there +// isn't any implicit conversion). +class ImplicitConversionInLoopCheck : public ClangTidyCheck { +public: + ImplicitConversionInLoopCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + void ReportAndFix(const ASTContext *Context, const VarDecl *VD, + const CXXOperatorCallExpr *OperatorCall); +}; + +} // namespace performance +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_IMPLICIT_CONVERSION_IN_LOOP_CHECK_H_ Index: clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/performance/ImplicitConversionInLoopCheck.cpp @@ -0,0 +1,98 @@ +//===--- ImplicitConversionInLoopCheck.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 "ImplicitConversionInLoopCheck.h" + +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace performance { + +// Checks if the stmt is a ImplicitCastExpr with a CastKind that is not a NoOp. +// The subtelty is that in some cases (user defined conversions), we can +// get to ImplicitCastExpr inside each other, with the outer one a NoOp. In this +// case we skip the first cast expr. +static bool IsNonTrivialImplicitCast(const Stmt *ST) { + if (const auto *ICE = dyn_cast(ST)) { + return (ICE->getCastKind() != CK_NoOp) || + IsNonTrivialImplicitCast(ICE->getSubExpr()); + } + return false; +} + +void ImplicitConversionInLoopCheck::registerMatchers(MatchFinder *Finder) { + // We look for const ref loop variables that (optionally inside an + // ExprWithCleanup) materialize a temporary, and contain a implicit + // conversion. The check on the implicit conversion is done in check() because + // we can't access implicit conversion subnode via matchers: has() skips casts + // and materialize! We also bind on the call to operator* to get the proper + // type in the diagnostic message. + // + // Note that when the implicit conversion is done through a user defined + // conversion operator, the node is a CXXMemberCallExpr, not a + // CXXOperatorCallExpr, so it should not get caught by the + // cxxOperatorCallExpr() matcher. + Finder->addMatcher( + cxxForRangeStmt(hasLoopVariable( + varDecl(hasType(qualType(references(qualType(isConstQualified())))), + hasInitializer(expr(hasDescendant(cxxOperatorCallExpr().bind( + "operator-call"))) + .bind("init"))) + .bind("faulty-var"))), + this); +} + +void ImplicitConversionInLoopCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *VD = Result.Nodes.getNodeAs("faulty-var"); + const auto *Init = Result.Nodes.getNodeAs("init"); + const auto *OperatorCall = + Result.Nodes.getNodeAs("operator-call"); + + if (const auto *Cleanup = dyn_cast(Init)) + Init = Cleanup->getSubExpr(); + + const auto *Materialized = dyn_cast(Init); + if (!Materialized) + return; + + // We ignore NoOp casts. Those are generated if the * operator on the + // iterator returns a value instead of a reference, and the loop variable + // is a reference. This situation is fine (it probably produces the same + // code at the end). + if (IsNonTrivialImplicitCast(Materialized->getTemporary())) + ReportAndFix(Result.Context, VD, OperatorCall); +} + +void ImplicitConversionInLoopCheck::ReportAndFix( + const ASTContext *Context, const VarDecl *VD, + const CXXOperatorCallExpr *OperatorCall) { + // We only match on const ref, so we should print a const ref version of the + // type. + QualType ConstType = OperatorCall->getType().withConst(); + QualType ConstRefType = Context->getLValueReferenceType(ConstType); + const char Message[] = + "the type of the loop variable %0 is different from the one returned " + "by the iterator and generates an implicit conversion; you can either " + "change the type to the matching one (%1 but 'const auto&' is always a " + "valid option) or remove the reference to make it explicit that you are " + "creating a new value"; + diag(VD->getLocStart(), Message) << VD << ConstRefType; +} + +} // namespace performance +} // namespace tidy +} // namespace clang Index: clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp @@ -12,7 +12,7 @@ #include "../ClangTidyModuleRegistry.h" #include "FasterStringFindCheck.h" #include "ForRangeCopyCheck.h" -#include "ImplicitCastInLoopCheck.h" +#include "ImplicitConversionInLoopCheck.h" #include "InefficientStringConcatenationCheck.h" #include "InefficientVectorOperationCheck.h" #include "TypePromotionInMathFnCheck.h" @@ -30,8 +30,8 @@ "performance-faster-string-find"); CheckFactories.registerCheck( "performance-for-range-copy"); - CheckFactories.registerCheck( - "performance-implicit-cast-in-loop"); + CheckFactories.registerCheck( + "performance-implicit-conversion-in-loop"); CheckFactories.registerCheck( "performance-inefficient-string-concatenation"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt @@ -9,7 +9,7 @@ ElseAfterReturnCheck.cpp FunctionSizeCheck.cpp IdentifierNamingCheck.cpp - ImplicitBoolCastCheck.cpp + ImplicitBoolConversionCheck.cpp InconsistentDeclarationParameterNameCheck.cpp MisleadingIndentationCheck.cpp MisplacedArrayIndexCheck.cpp Index: clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.h +++ clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.h @@ -1,46 +0,0 @@ -//===--- ImplicitBoolCastCheck.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_IMPLICIT_BOOL_CAST_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IMPLICIT_BOOL_CAST_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace readability { - -/// \brief Checks for use of implicit bool casts in expressions. -/// -/// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/readability-implicit-bool-cast.html -class ImplicitBoolCastCheck : public ClangTidyCheck { -public: - ImplicitBoolCastCheck(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; - -private: - void handleCastToBool(const ImplicitCastExpr *CastExpression, - const Stmt *ParentStatement, ASTContext &Context); - void handleCastFromBool(const ImplicitCastExpr *CastExpression, - const ImplicitCastExpr *FurtherImplicitCastExpression, - ASTContext &Context); - - bool AllowConditionalIntegerCasts; - bool AllowConditionalPointerCasts; -}; - -} // namespace readability -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IMPLICIT_BOOL_CAST_H Index: clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolCastCheck.cpp @@ -1,390 +0,0 @@ -//===--- ImplicitBoolCastCheck.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 "ImplicitBoolCastCheck.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Lex/Lexer.h" -#include "clang/Tooling/FixIt.h" -#include - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace readability { - -namespace { - -AST_MATCHER(Stmt, isMacroExpansion) { - SourceManager &SM = Finder->getASTContext().getSourceManager(); - SourceLocation Loc = Node.getLocStart(); - return SM.isMacroBodyExpansion(Loc) || SM.isMacroArgExpansion(Loc); -} - -bool isNULLMacroExpansion(const Stmt *Statement, ASTContext &Context) { - SourceManager &SM = Context.getSourceManager(); - const LangOptions &LO = Context.getLangOpts(); - SourceLocation Loc = Statement->getLocStart(); - return SM.isMacroBodyExpansion(Loc) && - Lexer::getImmediateMacroName(Loc, SM, LO) == "NULL"; -} - -AST_MATCHER(Stmt, isNULLMacroExpansion) { - return isNULLMacroExpansion(&Node, Finder->getASTContext()); -} - -StringRef getZeroLiteralToCompareWithForType(CastKind CastExprKind, - QualType Type, - ASTContext &Context) { - switch (CastExprKind) { - case CK_IntegralToBoolean: - return Type->isUnsignedIntegerType() ? "0u" : "0"; - - case CK_FloatingToBoolean: - return Context.hasSameType(Type, Context.FloatTy) ? "0.0f" : "0.0"; - - case CK_PointerToBoolean: - case CK_MemberPointerToBoolean: // Fall-through on purpose. - return Context.getLangOpts().CPlusPlus11 ? "nullptr" : "0"; - - default: - llvm_unreachable("Unexpected cast kind"); - } -} - -bool isUnaryLogicalNotOperator(const Stmt *Statement) { - const auto *UnaryOperatorExpr = dyn_cast(Statement); - return UnaryOperatorExpr && UnaryOperatorExpr->getOpcode() == UO_LNot; -} - -bool areParensNeededForOverloadedOperator(OverloadedOperatorKind OperatorKind) { - switch (OperatorKind) { - case OO_New: - case OO_Delete: // Fall-through on purpose. - case OO_Array_New: - case OO_Array_Delete: - case OO_ArrowStar: - case OO_Arrow: - case OO_Call: - case OO_Subscript: - return false; - - default: - return true; - } -} - -bool areParensNeededForStatement(const Stmt *Statement) { - if (const auto *OperatorCall = dyn_cast(Statement)) { - return areParensNeededForOverloadedOperator(OperatorCall->getOperator()); - } - - return isa(Statement) || isa(Statement); -} - -void fixGenericExprCastToBool(DiagnosticBuilder &Diag, - const ImplicitCastExpr *Cast, const Stmt *Parent, - ASTContext &Context) { - // In case of expressions like (! integer), we should remove the redundant not - // operator and use inverted comparison (integer == 0). - bool InvertComparison = - Parent != nullptr && isUnaryLogicalNotOperator(Parent); - if (InvertComparison) { - SourceLocation ParentStartLoc = Parent->getLocStart(); - SourceLocation ParentEndLoc = - cast(Parent)->getSubExpr()->getLocStart(); - Diag << FixItHint::CreateRemoval( - CharSourceRange::getCharRange(ParentStartLoc, ParentEndLoc)); - - Parent = Context.getParents(*Parent)[0].get(); - } - - const Expr *SubExpr = Cast->getSubExpr(); - - bool NeedInnerParens = areParensNeededForStatement(SubExpr); - bool NeedOuterParens = - Parent != nullptr && areParensNeededForStatement(Parent); - - std::string StartLocInsertion; - - if (NeedOuterParens) { - StartLocInsertion += "("; - } - if (NeedInnerParens) { - StartLocInsertion += "("; - } - - if (!StartLocInsertion.empty()) { - Diag << FixItHint::CreateInsertion(Cast->getLocStart(), StartLocInsertion); - } - - std::string EndLocInsertion; - - if (NeedInnerParens) { - EndLocInsertion += ")"; - } - - if (InvertComparison) { - EndLocInsertion += " == "; - } else { - EndLocInsertion += " != "; - } - - EndLocInsertion += getZeroLiteralToCompareWithForType( - Cast->getCastKind(), SubExpr->getType(), Context); - - if (NeedOuterParens) { - EndLocInsertion += ")"; - } - - SourceLocation EndLoc = Lexer::getLocForEndOfToken( - Cast->getLocEnd(), 0, Context.getSourceManager(), Context.getLangOpts()); - Diag << FixItHint::CreateInsertion(EndLoc, EndLocInsertion); -} - -StringRef getEquivalentBoolLiteralForExpr(const Expr *Expression, - ASTContext &Context) { - if (isNULLMacroExpansion(Expression, Context)) { - return "false"; - } - - if (const auto *IntLit = dyn_cast(Expression)) { - return (IntLit->getValue() == 0) ? "false" : "true"; - } - - if (const auto *FloatLit = dyn_cast(Expression)) { - llvm::APFloat FloatLitAbsValue = FloatLit->getValue(); - FloatLitAbsValue.clearSign(); - return (FloatLitAbsValue.bitcastToAPInt() == 0) ? "false" : "true"; - } - - if (const auto *CharLit = dyn_cast(Expression)) { - return (CharLit->getValue() == 0) ? "false" : "true"; - } - - if (isa(Expression->IgnoreCasts())) { - return "true"; - } - - return StringRef(); -} - -void fixGenericExprCastFromBool(DiagnosticBuilder &Diag, - const ImplicitCastExpr *Cast, - ASTContext &Context, StringRef OtherType) { - const Expr *SubExpr = Cast->getSubExpr(); - bool NeedParens = !isa(SubExpr); - - Diag << FixItHint::CreateInsertion( - Cast->getLocStart(), - (Twine("static_cast<") + OtherType + ">" + (NeedParens ? "(" : "")) - .str()); - - if (NeedParens) { - SourceLocation EndLoc = Lexer::getLocForEndOfToken( - Cast->getLocEnd(), 0, Context.getSourceManager(), - Context.getLangOpts()); - - Diag << FixItHint::CreateInsertion(EndLoc, ")"); - } -} - -StringRef getEquivalentForBoolLiteral(const CXXBoolLiteralExpr *BoolLiteral, - QualType DestType, ASTContext &Context) { - // Prior to C++11, false literal could be implicitly converted to pointer. - if (!Context.getLangOpts().CPlusPlus11 && - (DestType->isPointerType() || DestType->isMemberPointerType()) && - BoolLiteral->getValue() == false) { - return "0"; - } - - if (DestType->isFloatingType()) { - if (Context.hasSameType(DestType, Context.FloatTy)) { - return BoolLiteral->getValue() ? "1.0f" : "0.0f"; - } - return BoolLiteral->getValue() ? "1.0" : "0.0"; - } - - if (DestType->isUnsignedIntegerType()) { - return BoolLiteral->getValue() ? "1u" : "0u"; - } - return BoolLiteral->getValue() ? "1" : "0"; -} - -bool isAllowedConditionalCast(const ImplicitCastExpr *Cast, - ASTContext &Context) { - std::queue Q; - Q.push(Cast); - while (!Q.empty()) { - for (const auto &N : Context.getParents(*Q.front())) { - const Stmt *S = N.get(); - if (!S) - return false; - if (isa(S) || isa(S) || isa(S) || - isa(S) || isa(S)) - return true; - if (isa(S) || isa(S) || - isUnaryLogicalNotOperator(S) || - (isa(S) && cast(S)->isLogicalOp())) { - Q.push(S); - } else { - return false; - } - } - Q.pop(); - } - return false; -} - -} // anonymous namespace - -ImplicitBoolCastCheck::ImplicitBoolCastCheck(StringRef Name, - ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - AllowConditionalIntegerCasts( - Options.get("AllowConditionalIntegerCasts", false)), - AllowConditionalPointerCasts( - Options.get("AllowConditionalPointerCasts", false)) {} - -void ImplicitBoolCastCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "AllowConditionalIntegerCasts", - AllowConditionalIntegerCasts); - Options.store(Opts, "AllowConditionalPointerCasts", - AllowConditionalPointerCasts); -} - -void ImplicitBoolCastCheck::registerMatchers(MatchFinder *Finder) { - // This check doesn't make much sense if we run it on language without - // built-in bool support. - if (!getLangOpts().Bool) { - return; - } - - auto exceptionCases = - expr(anyOf(allOf(isMacroExpansion(), unless(isNULLMacroExpansion())), - hasParent(explicitCastExpr()))); - auto implicitCastFromBool = implicitCastExpr( - anyOf(hasCastKind(CK_IntegralCast), hasCastKind(CK_IntegralToFloating), - // Prior to C++11 cast from bool literal to pointer was allowed. - allOf(anyOf(hasCastKind(CK_NullToPointer), - hasCastKind(CK_NullToMemberPointer)), - hasSourceExpression(cxxBoolLiteral()))), - hasSourceExpression(expr(hasType(booleanType()))), - unless(exceptionCases)); - auto boolXor = - binaryOperator(hasOperatorName("^"), hasLHS(implicitCastFromBool), - hasRHS(implicitCastFromBool)); - Finder->addMatcher( - implicitCastExpr( - anyOf(hasCastKind(CK_IntegralToBoolean), - hasCastKind(CK_FloatingToBoolean), - hasCastKind(CK_PointerToBoolean), - hasCastKind(CK_MemberPointerToBoolean)), - // Exclude case of using if or while statements with variable - // declaration, e.g.: - // if (int var = functionCall()) {} - unless( - hasParent(stmt(anyOf(ifStmt(), whileStmt()), has(declStmt())))), - // Exclude cases common to implicit cast to and from bool. - unless(exceptionCases), unless(has(boolXor)), - // Retrive also parent statement, to check if we need additional - // parens in replacement. - anyOf(hasParent(stmt().bind("parentStmt")), anything()), - unless(isInTemplateInstantiation()), - unless(hasAncestor(functionTemplateDecl()))) - .bind("implicitCastToBool"), - this); - - auto boolComparison = binaryOperator( - anyOf(hasOperatorName("=="), hasOperatorName("!=")), - hasLHS(implicitCastFromBool), hasRHS(implicitCastFromBool)); - auto boolOpAssignment = - binaryOperator(anyOf(hasOperatorName("|="), hasOperatorName("&=")), - hasLHS(expr(hasType(booleanType())))); - Finder->addMatcher( - implicitCastExpr( - implicitCastFromBool, - // Exclude comparisons of bools, as they are always cast to integers - // in such context: - // bool_expr_a == bool_expr_b - // bool_expr_a != bool_expr_b - unless(hasParent(binaryOperator( - anyOf(boolComparison, boolXor, boolOpAssignment)))), - // Check also for nested casts, for example: bool -> int -> float. - anyOf(hasParent(implicitCastExpr().bind("furtherImplicitCast")), - anything()), - unless(isInTemplateInstantiation()), - unless(hasAncestor(functionTemplateDecl()))) - .bind("implicitCastFromBool"), - this); -} - -void ImplicitBoolCastCheck::check(const MatchFinder::MatchResult &Result) { - if (const auto *CastToBool = - Result.Nodes.getNodeAs("implicitCastToBool")) { - const auto *Parent = Result.Nodes.getNodeAs("parentStmt"); - return handleCastToBool(CastToBool, Parent, *Result.Context); - } - - if (const auto *CastFromBool = - Result.Nodes.getNodeAs("implicitCastFromBool")) { - const auto *NextImplicitCast = - Result.Nodes.getNodeAs("furtherImplicitCast"); - return handleCastFromBool(CastFromBool, NextImplicitCast, *Result.Context); - } -} - -void ImplicitBoolCastCheck::handleCastToBool(const ImplicitCastExpr *Cast, - const Stmt *Parent, - ASTContext &Context) { - if (AllowConditionalPointerCasts && - (Cast->getCastKind() == CK_PointerToBoolean || - Cast->getCastKind() == CK_MemberPointerToBoolean) && - isAllowedConditionalCast(Cast, Context)) { - return; - } - - if (AllowConditionalIntegerCasts && - Cast->getCastKind() == CK_IntegralToBoolean && - isAllowedConditionalCast(Cast, Context)) { - return; - } - - auto Diag = diag(Cast->getLocStart(), "implicit cast %0 -> bool") - << Cast->getSubExpr()->getType(); - - StringRef EquivalentLiteral = - getEquivalentBoolLiteralForExpr(Cast->getSubExpr(), Context); - if (!EquivalentLiteral.empty()) { - Diag << tooling::fixit::createReplacement(*Cast, EquivalentLiteral); - } else { - fixGenericExprCastToBool(Diag, Cast, Parent, Context); - } -} - -void ImplicitBoolCastCheck::handleCastFromBool( - const ImplicitCastExpr *Cast, const ImplicitCastExpr *NextImplicitCast, - ASTContext &Context) { - QualType DestType = - NextImplicitCast ? NextImplicitCast->getType() : Cast->getType(); - auto Diag = diag(Cast->getLocStart(), "implicit cast bool -> %0") << DestType; - - if (const auto *BoolLiteral = - dyn_cast(Cast->getSubExpr())) { - Diag << tooling::fixit::createReplacement( - *Cast, getEquivalentForBoolLiteral(BoolLiteral, DestType, Context)); - } else { - fixGenericExprCastFromBool(Diag, Cast, Context, DestType.getAsString()); - } -} - -} // namespace readability -} // namespace tidy -} // namespace clang Index: clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolConversionCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolConversionCheck.h +++ clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolConversionCheck.h @@ -0,0 +1,46 @@ +//===--- ImplicitBoolConversionCheck.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_IMPLICIT_BOOL_CONVERSION_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IMPLICIT_BOOL_CONVERSION_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// \brief Checks for use of implicit bool conversions in expressions. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-implicit-bool-conversion.html +class ImplicitBoolConversionCheck : public ClangTidyCheck { +public: + ImplicitBoolConversionCheck(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; + +private: + void handleCastToBool(const ImplicitCastExpr *CastExpression, + const Stmt *ParentStatement, ASTContext &Context); + void handleCastFromBool(const ImplicitCastExpr *CastExpression, + const ImplicitCastExpr *FurtherImplicitCastExpression, + ASTContext &Context); + + const bool AllowIntegerConditions; + const bool AllowPointerConditions; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IMPLICIT_BOOL_CONVERSION_H Index: clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolConversionCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolConversionCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/readability/ImplicitBoolConversionCheck.cpp @@ -0,0 +1,388 @@ +//===--- ImplicitBoolConversionCheck.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 "ImplicitBoolConversionCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "clang/Tooling/FixIt.h" +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +namespace { + +AST_MATCHER(Stmt, isMacroExpansion) { + SourceManager &SM = Finder->getASTContext().getSourceManager(); + SourceLocation Loc = Node.getLocStart(); + return SM.isMacroBodyExpansion(Loc) || SM.isMacroArgExpansion(Loc); +} + +bool isNULLMacroExpansion(const Stmt *Statement, ASTContext &Context) { + SourceManager &SM = Context.getSourceManager(); + const LangOptions &LO = Context.getLangOpts(); + SourceLocation Loc = Statement->getLocStart(); + return SM.isMacroBodyExpansion(Loc) && + Lexer::getImmediateMacroName(Loc, SM, LO) == "NULL"; +} + +AST_MATCHER(Stmt, isNULLMacroExpansion) { + return isNULLMacroExpansion(&Node, Finder->getASTContext()); +} + +StringRef getZeroLiteralToCompareWithForType(CastKind CastExprKind, + QualType Type, + ASTContext &Context) { + switch (CastExprKind) { + case CK_IntegralToBoolean: + return Type->isUnsignedIntegerType() ? "0u" : "0"; + + case CK_FloatingToBoolean: + return Context.hasSameType(Type, Context.FloatTy) ? "0.0f" : "0.0"; + + case CK_PointerToBoolean: + case CK_MemberPointerToBoolean: // Fall-through on purpose. + return Context.getLangOpts().CPlusPlus11 ? "nullptr" : "0"; + + default: + llvm_unreachable("Unexpected cast kind"); + } +} + +bool isUnaryLogicalNotOperator(const Stmt *Statement) { + const auto *UnaryOperatorExpr = dyn_cast(Statement); + return UnaryOperatorExpr && UnaryOperatorExpr->getOpcode() == UO_LNot; +} + +bool areParensNeededForOverloadedOperator(OverloadedOperatorKind OperatorKind) { + switch (OperatorKind) { + case OO_New: + case OO_Delete: // Fall-through on purpose. + case OO_Array_New: + case OO_Array_Delete: + case OO_ArrowStar: + case OO_Arrow: + case OO_Call: + case OO_Subscript: + return false; + + default: + return true; + } +} + +bool areParensNeededForStatement(const Stmt *Statement) { + if (const auto *OperatorCall = dyn_cast(Statement)) { + return areParensNeededForOverloadedOperator(OperatorCall->getOperator()); + } + + return isa(Statement) || isa(Statement); +} + +void fixGenericExprCastToBool(DiagnosticBuilder &Diag, + const ImplicitCastExpr *Cast, const Stmt *Parent, + ASTContext &Context) { + // In case of expressions like (! integer), we should remove the redundant not + // operator and use inverted comparison (integer == 0). + bool InvertComparison = + Parent != nullptr && isUnaryLogicalNotOperator(Parent); + if (InvertComparison) { + SourceLocation ParentStartLoc = Parent->getLocStart(); + SourceLocation ParentEndLoc = + cast(Parent)->getSubExpr()->getLocStart(); + Diag << FixItHint::CreateRemoval( + CharSourceRange::getCharRange(ParentStartLoc, ParentEndLoc)); + + Parent = Context.getParents(*Parent)[0].get(); + } + + const Expr *SubExpr = Cast->getSubExpr(); + + bool NeedInnerParens = areParensNeededForStatement(SubExpr); + bool NeedOuterParens = + Parent != nullptr && areParensNeededForStatement(Parent); + + std::string StartLocInsertion; + + if (NeedOuterParens) { + StartLocInsertion += "("; + } + if (NeedInnerParens) { + StartLocInsertion += "("; + } + + if (!StartLocInsertion.empty()) { + Diag << FixItHint::CreateInsertion(Cast->getLocStart(), StartLocInsertion); + } + + std::string EndLocInsertion; + + if (NeedInnerParens) { + EndLocInsertion += ")"; + } + + if (InvertComparison) { + EndLocInsertion += " == "; + } else { + EndLocInsertion += " != "; + } + + EndLocInsertion += getZeroLiteralToCompareWithForType( + Cast->getCastKind(), SubExpr->getType(), Context); + + if (NeedOuterParens) { + EndLocInsertion += ")"; + } + + SourceLocation EndLoc = Lexer::getLocForEndOfToken( + Cast->getLocEnd(), 0, Context.getSourceManager(), Context.getLangOpts()); + Diag << FixItHint::CreateInsertion(EndLoc, EndLocInsertion); +} + +StringRef getEquivalentBoolLiteralForExpr(const Expr *Expression, + ASTContext &Context) { + if (isNULLMacroExpansion(Expression, Context)) { + return "false"; + } + + if (const auto *IntLit = dyn_cast(Expression)) { + return (IntLit->getValue() == 0) ? "false" : "true"; + } + + if (const auto *FloatLit = dyn_cast(Expression)) { + llvm::APFloat FloatLitAbsValue = FloatLit->getValue(); + FloatLitAbsValue.clearSign(); + return (FloatLitAbsValue.bitcastToAPInt() == 0) ? "false" : "true"; + } + + if (const auto *CharLit = dyn_cast(Expression)) { + return (CharLit->getValue() == 0) ? "false" : "true"; + } + + if (isa(Expression->IgnoreCasts())) { + return "true"; + } + + return StringRef(); +} + +void fixGenericExprCastFromBool(DiagnosticBuilder &Diag, + const ImplicitCastExpr *Cast, + ASTContext &Context, StringRef OtherType) { + const Expr *SubExpr = Cast->getSubExpr(); + bool NeedParens = !isa(SubExpr); + + Diag << FixItHint::CreateInsertion( + Cast->getLocStart(), + (Twine("static_cast<") + OtherType + ">" + (NeedParens ? "(" : "")) + .str()); + + if (NeedParens) { + SourceLocation EndLoc = Lexer::getLocForEndOfToken( + Cast->getLocEnd(), 0, Context.getSourceManager(), + Context.getLangOpts()); + + Diag << FixItHint::CreateInsertion(EndLoc, ")"); + } +} + +StringRef getEquivalentForBoolLiteral(const CXXBoolLiteralExpr *BoolLiteral, + QualType DestType, ASTContext &Context) { + // Prior to C++11, false literal could be implicitly converted to pointer. + if (!Context.getLangOpts().CPlusPlus11 && + (DestType->isPointerType() || DestType->isMemberPointerType()) && + BoolLiteral->getValue() == false) { + return "0"; + } + + if (DestType->isFloatingType()) { + if (Context.hasSameType(DestType, Context.FloatTy)) { + return BoolLiteral->getValue() ? "1.0f" : "0.0f"; + } + return BoolLiteral->getValue() ? "1.0" : "0.0"; + } + + if (DestType->isUnsignedIntegerType()) { + return BoolLiteral->getValue() ? "1u" : "0u"; + } + return BoolLiteral->getValue() ? "1" : "0"; +} + +bool isCastAllowedInCondition(const ImplicitCastExpr *Cast, + ASTContext &Context) { + std::queue Q; + Q.push(Cast); + while (!Q.empty()) { + for (const auto &N : Context.getParents(*Q.front())) { + const Stmt *S = N.get(); + if (!S) + return false; + if (isa(S) || isa(S) || isa(S) || + isa(S) || isa(S)) + return true; + if (isa(S) || isa(S) || + isUnaryLogicalNotOperator(S) || + (isa(S) && cast(S)->isLogicalOp())) { + Q.push(S); + } else { + return false; + } + } + Q.pop(); + } + return false; +} + +} // anonymous namespace + +ImplicitBoolConversionCheck::ImplicitBoolConversionCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AllowIntegerConditions(Options.get("AllowIntegerConditions", false)), + AllowPointerConditions(Options.get("AllowPointerConditions", false)) {} + +void ImplicitBoolConversionCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowIntegerConditions", AllowIntegerConditions); + Options.store(Opts, "AllowPointerConditions", AllowPointerConditions); +} + +void ImplicitBoolConversionCheck::registerMatchers(MatchFinder *Finder) { + // This check doesn't make much sense if we run it on language without + // built-in bool support. + if (!getLangOpts().Bool) { + return; + } + + auto exceptionCases = + expr(anyOf(allOf(isMacroExpansion(), unless(isNULLMacroExpansion())), + hasParent(explicitCastExpr()))); + auto implicitCastFromBool = implicitCastExpr( + anyOf(hasCastKind(CK_IntegralCast), hasCastKind(CK_IntegralToFloating), + // Prior to C++11 cast from bool literal to pointer was allowed. + allOf(anyOf(hasCastKind(CK_NullToPointer), + hasCastKind(CK_NullToMemberPointer)), + hasSourceExpression(cxxBoolLiteral()))), + hasSourceExpression(expr(hasType(booleanType()))), + unless(exceptionCases)); + auto boolXor = + binaryOperator(hasOperatorName("^"), hasLHS(implicitCastFromBool), + hasRHS(implicitCastFromBool)); + Finder->addMatcher( + implicitCastExpr( + anyOf(hasCastKind(CK_IntegralToBoolean), + hasCastKind(CK_FloatingToBoolean), + hasCastKind(CK_PointerToBoolean), + hasCastKind(CK_MemberPointerToBoolean)), + // Exclude case of using if or while statements with variable + // declaration, e.g.: + // if (int var = functionCall()) {} + unless( + hasParent(stmt(anyOf(ifStmt(), whileStmt()), has(declStmt())))), + // Exclude cases common to implicit cast to and from bool. + unless(exceptionCases), unless(has(boolXor)), + // Retrive also parent statement, to check if we need additional + // parens in replacement. + anyOf(hasParent(stmt().bind("parentStmt")), anything()), + unless(isInTemplateInstantiation()), + unless(hasAncestor(functionTemplateDecl()))) + .bind("implicitCastToBool"), + this); + + auto boolComparison = binaryOperator( + anyOf(hasOperatorName("=="), hasOperatorName("!=")), + hasLHS(implicitCastFromBool), hasRHS(implicitCastFromBool)); + auto boolOpAssignment = + binaryOperator(anyOf(hasOperatorName("|="), hasOperatorName("&=")), + hasLHS(expr(hasType(booleanType())))); + Finder->addMatcher( + implicitCastExpr( + implicitCastFromBool, + // Exclude comparisons of bools, as they are always cast to integers + // in such context: + // bool_expr_a == bool_expr_b + // bool_expr_a != bool_expr_b + unless(hasParent(binaryOperator( + anyOf(boolComparison, boolXor, boolOpAssignment)))), + // Check also for nested casts, for example: bool -> int -> float. + anyOf(hasParent(implicitCastExpr().bind("furtherImplicitCast")), + anything()), + unless(isInTemplateInstantiation()), + unless(hasAncestor(functionTemplateDecl()))) + .bind("implicitCastFromBool"), + this); +} + +void ImplicitBoolConversionCheck::check( + const MatchFinder::MatchResult &Result) { + if (const auto *CastToBool = + Result.Nodes.getNodeAs("implicitCastToBool")) { + const auto *Parent = Result.Nodes.getNodeAs("parentStmt"); + return handleCastToBool(CastToBool, Parent, *Result.Context); + } + + if (const auto *CastFromBool = + Result.Nodes.getNodeAs("implicitCastFromBool")) { + const auto *NextImplicitCast = + Result.Nodes.getNodeAs("furtherImplicitCast"); + return handleCastFromBool(CastFromBool, NextImplicitCast, *Result.Context); + } +} + +void ImplicitBoolConversionCheck::handleCastToBool(const ImplicitCastExpr *Cast, + const Stmt *Parent, + ASTContext &Context) { + if (AllowPointerConditions && + (Cast->getCastKind() == CK_PointerToBoolean || + Cast->getCastKind() == CK_MemberPointerToBoolean) && + isCastAllowedInCondition(Cast, Context)) { + return; + } + + if (AllowIntegerConditions && Cast->getCastKind() == CK_IntegralToBoolean && + isCastAllowedInCondition(Cast, Context)) { + return; + } + + auto Diag = diag(Cast->getLocStart(), "implicit conversion %0 -> bool") + << Cast->getSubExpr()->getType(); + + StringRef EquivalentLiteral = + getEquivalentBoolLiteralForExpr(Cast->getSubExpr(), Context); + if (!EquivalentLiteral.empty()) { + Diag << tooling::fixit::createReplacement(*Cast, EquivalentLiteral); + } else { + fixGenericExprCastToBool(Diag, Cast, Parent, Context); + } +} + +void ImplicitBoolConversionCheck::handleCastFromBool( + const ImplicitCastExpr *Cast, const ImplicitCastExpr *NextImplicitCast, + ASTContext &Context) { + QualType DestType = + NextImplicitCast ? NextImplicitCast->getType() : Cast->getType(); + auto Diag = diag(Cast->getLocStart(), "implicit conversion bool -> %0") + << DestType; + + if (const auto *BoolLiteral = + dyn_cast(Cast->getSubExpr())) { + Diag << tooling::fixit::createReplacement( + *Cast, getEquivalentForBoolLiteral(BoolLiteral, DestType, Context)); + } else { + fixGenericExprCastFromBool(Diag, Cast, Context, DestType.getAsString()); + } +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -18,7 +18,7 @@ #include "ElseAfterReturnCheck.h" #include "FunctionSizeCheck.h" #include "IdentifierNamingCheck.h" -#include "ImplicitBoolCastCheck.h" +#include "ImplicitBoolConversionCheck.h" #include "InconsistentDeclarationParameterNameCheck.h" #include "MisleadingIndentationCheck.h" #include "MisplacedArrayIndexCheck.h" @@ -58,8 +58,8 @@ "readability-function-size"); CheckFactories.registerCheck( "readability-identifier-naming"); - CheckFactories.registerCheck( - "readability-implicit-bool-cast"); + CheckFactories.registerCheck( + "readability-implicit-bool-conversion"); CheckFactories.registerCheck( "readability-inconsistent-declaration-parameter-name"); CheckFactories.registerCheck( Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst @@ -57,7 +57,19 @@ Improvements to clang-tidy -------------------------- -The improvements are... +* Renamed checks to use correct term "implicit conversion" instead of "implicit + cast" and modified messages and option names accordingly: + + - **performance-implicit-cast-in-loop** was renamed to + `performance-implicit-conversion-in-loop + `_ + - **readability-implicit-bool-cast** was renamed to + `readability-implicit-bool-conversion + `_; + the check's options were renamed as follows: + ``AllowConditionalIntegerCasts`` -> ``AllowIntegerConditions``, + ``AllowConditionalPointerCasts`` -> ``AllowPointerConditions``. + Improvements to include-fixer ----------------------------- Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -150,7 +150,7 @@ mpi-type-mismatch performance-faster-string-find performance-for-range-copy - performance-implicit-cast-in-loop + performance-implicit-conversion-in-loop performance-inefficient-string-concatenation performance-inefficient-vector-operation performance-type-promotion-in-math-fn @@ -164,7 +164,7 @@ readability-else-after-return readability-function-size readability-identifier-naming - readability-implicit-bool-cast + readability-implicit-bool-conversion readability-inconsistent-declaration-parameter-name readability-misleading-indentation readability-misplaced-array-index Index: clang-tools-extra/trunk/docs/clang-tidy/checks/performance-implicit-cast-in-loop.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/performance-implicit-cast-in-loop.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/performance-implicit-cast-in-loop.rst @@ -1,21 +1,12 @@ +:orphan: + .. title:: clang-tidy - performance-implicit-cast-in-loop +.. meta:: + :http-equiv=refresh: 5;URL=performance-implicit-conversion-in-loop.html performance-implicit-cast-in-loop ================================= -This warning appears in a range-based loop with a loop variable of const ref -type where the type of the variable does not match the one returned by the -iterator. This means that an implicit cast has been added, which can for example -result in expensive deep copies. - -Example: - -.. code-block:: c++ - - map> my_map; - for (const pair>& p : my_map) {} - // The iterator type is in fact pair>, which means - // that the compiler added a cast, resulting in a copy of the vectors. +This check has been renamed to `performance-implicit-conversion-in-loop +`_. -The easiest solution is usually to use ``const auto&`` instead of writing the type -manually. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/performance-implicit-conversion-in-loop.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/performance-implicit-conversion-in-loop.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/performance-implicit-conversion-in-loop.rst @@ -0,0 +1,21 @@ +.. title:: clang-tidy - performance-implicit-conversion-in-loop + +performance-implicit-conversion-in-loop +======================================= + +This warning appears in a range-based loop with a loop variable of const ref +type where the type of the variable does not match the one returned by the +iterator. This means that an implicit conversion happens, which can for example +result in expensive deep copies. + +Example: + +.. code-block:: c++ + + map> my_map; + for (const pair>& p : my_map) {} + // The iterator type is in fact pair>, which means + // that the compiler added a conversion, resulting in a copy of the vectors. + +The easiest solution is usually to use ``const auto&`` instead of writing the +type manually. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-implicit-bool-cast.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-implicit-bool-cast.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-implicit-bool-cast.rst @@ -1,130 +1,11 @@ +:orphan: + .. title:: clang-tidy - readability-implicit-bool-cast +.. meta:: + :http-equiv=refresh: 5;URL=readability-implicit-bool-conversion.html readability-implicit-bool-cast ============================== -This check can be used to find implicit conversions between built-in types and -booleans. Depending on use case, it may simply help with readability of the code, -or in some cases, point to potential bugs which remain unnoticed due to implicit -conversions. - -The following is a real-world example of bug which was hiding behind implicit -``bool`` cast: - -.. code-block:: c++ - - class Foo { - int m_foo; - - public: - void setFoo(bool foo) { m_foo = foo; } // warning: implicit cast bool -> int - int getFoo() { return m_foo; } - }; - - void use(Foo& foo) { - bool value = foo.getFoo(); // warning: implicit cast int -> bool - } - -This code is the result of unsuccessful refactoring, where type of ``m_foo`` -changed from ``bool`` to ``int``. The programmer forgot to change all -occurrences of ``bool``, and the remaining code is no longer correct, yet it -still compiles without any visible warnings. - -In addition to issuing warnings, fix-it hints are provided to help solve the -reported issues. This can be used for improving readability of code, for -example: - -.. code-block:: c++ - - void conversionsToBool() { - float floating; - bool boolean = floating; - // ^ propose replacement: bool boolean = floating != 0.0f; - - int integer; - if (integer) {} - // ^ propose replacement: if (integer != 0) {} - - int* pointer; - if (!pointer) {} - // ^ propose replacement: if (pointer == nullptr) {} - - while (1) {} - // ^ propose replacement: while (true) {} - } - - void functionTakingInt(int param); - - void conversionsFromBool() { - bool boolean; - functionTakingInt(boolean); - // ^ propose replacement: functionTakingInt(static_cast(boolean)); - - functionTakingInt(true); - // ^ propose replacement: functionTakingInt(1); - } - -In general, the following cast types are checked: - -- integer expression/literal to boolean, - -- floating expression/literal to boolean, - -- pointer/pointer to member/``nullptr``/``NULL`` to boolean, - -- boolean expression/literal to integer, - -- boolean expression/literal to floating. - -The rules for generating fix-it hints are: - -- in case of casts from other built-in type to bool, an explicit comparison - is proposed to make it clear what exaclty is being compared: - - - ``bool boolean = floating;`` is changed to - ``bool boolean = floating == 0.0f;``, - - - for other types, appropriate literals are used (``0``, ``0u``, ``0.0f``, - ``0.0``, ``nullptr``), - -- in case of negated expressions cast to bool, the proposed replacement with - comparison is simplified: - - - ``if (!pointer)`` is changed to ``if (pointer == nullptr)``, - -- in case of casts from bool to other built-in types, an explicit ``static_cast`` - is proposed to make it clear that a cast is taking place: - - - ``int integer = boolean;`` is changed to - ``int integer = static_cast(boolean);``, - -- if the cast is performed on type literals, an equivalent literal is proposed, - according to what type is actually expected, for example: - - - ``functionTakingBool(0);`` is changed to ``functionTakingBool(false);``, - - - ``functionTakingInt(true);`` is changed to ``functionTakingInt(1);``, - - - for other types, appropriate literals are used (``false``, ``true``, ``0``, - ``1``, ``0u``, ``1u``, ``0.0f``, ``1.0f``, ``0.0``, ``1.0f``). - -Some additional accommodations are made for pre-C++11 dialects: - -- ``false`` literal cast to pointer is detected, - -- instead of ``nullptr`` literal, ``0`` is proposed as replacement. - -Occurrences of implicit casts inside macros and template instantiations are -deliberately ignored, as it is not clear how to deal with such cases. - -Options -------- - -.. option:: AllowConditionalIntegerCasts - - When non-zero, the check will allow conditional integer casts. Default is - `0`. - -.. option:: AllowConditionalPointerCasts - - When non-zero, the check will allow conditional pointer casts. Default is `0`. +This check has been renamed to `readability-implicit-bool-conversion +`_. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-implicit-bool-conversion.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-implicit-bool-conversion.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-implicit-bool-conversion.rst @@ -0,0 +1,132 @@ +.. title:: clang-tidy - readability-implicit-bool-conversion + +readability-implicit-bool-conversion +==================================== + +This check can be used to find implicit conversions between built-in types and +booleans. Depending on use case, it may simply help with readability of the code, +or in some cases, point to potential bugs which remain unnoticed due to implicit +conversions. + +The following is a real-world example of bug which was hiding behind implicit +``bool`` conversion: + +.. code-block:: c++ + + class Foo { + int m_foo; + + public: + void setFoo(bool foo) { m_foo = foo; } // warning: implicit conversion bool -> int + int getFoo() { return m_foo; } + }; + + void use(Foo& foo) { + bool value = foo.getFoo(); // warning: implicit conversion int -> bool + } + +This code is the result of unsuccessful refactoring, where type of ``m_foo`` +changed from ``bool`` to ``int``. The programmer forgot to change all +occurrences of ``bool``, and the remaining code is no longer correct, yet it +still compiles without any visible warnings. + +In addition to issuing warnings, fix-it hints are provided to help solve the +reported issues. This can be used for improving readability of code, for +example: + +.. code-block:: c++ + + void conversionsToBool() { + float floating; + bool boolean = floating; + // ^ propose replacement: bool boolean = floating != 0.0f; + + int integer; + if (integer) {} + // ^ propose replacement: if (integer != 0) {} + + int* pointer; + if (!pointer) {} + // ^ propose replacement: if (pointer == nullptr) {} + + while (1) {} + // ^ propose replacement: while (true) {} + } + + void functionTakingInt(int param); + + void conversionsFromBool() { + bool boolean; + functionTakingInt(boolean); + // ^ propose replacement: functionTakingInt(static_cast(boolean)); + + functionTakingInt(true); + // ^ propose replacement: functionTakingInt(1); + } + +In general, the following conversion types are checked: + +- integer expression/literal to boolean, + +- floating expression/literal to boolean, + +- pointer/pointer to member/``nullptr``/``NULL`` to boolean, + +- boolean expression/literal to integer, + +- boolean expression/literal to floating. + +The rules for generating fix-it hints are: + +- in case of conversions from other built-in type to bool, an explicit + comparison is proposed to make it clear what exaclty is being compared: + + - ``bool boolean = floating;`` is changed to + ``bool boolean = floating == 0.0f;``, + + - for other types, appropriate literals are used (``0``, ``0u``, ``0.0f``, + ``0.0``, ``nullptr``), + +- in case of negated expressions conversion to bool, the proposed replacement + with comparison is simplified: + + - ``if (!pointer)`` is changed to ``if (pointer == nullptr)``, + +- in case of conversions from bool to other built-in types, an explicit + ``static_cast`` is proposed to make it clear that a conversion is taking + place: + + - ``int integer = boolean;`` is changed to + ``int integer = static_cast(boolean);``, + +- if the conversion is performed on type literals, an equivalent literal is + proposed, according to what type is actually expected, for example: + + - ``functionTakingBool(0);`` is changed to ``functionTakingBool(false);``, + + - ``functionTakingInt(true);`` is changed to ``functionTakingInt(1);``, + + - for other types, appropriate literals are used (``false``, ``true``, ``0``, + ``1``, ``0u``, ``1u``, ``0.0f``, ``1.0f``, ``0.0``, ``1.0f``). + +Some additional accommodations are made for pre-C++11 dialects: + +- ``false`` literal conversion to pointer is detected, + +- instead of ``nullptr`` literal, ``0`` is proposed as replacement. + +Occurrences of implicit conversions inside macros and template instantiations +are deliberately ignored, as it is not clear how to deal with such cases. + +Options +------- + +.. option:: AllowIntegerConditions + + When non-zero, the check will allow conditional integer conversions. Default + is `0`. + +.. option:: AllowPointerConditions + + When non-zero, the check will allow conditional pointer conversions. Default + is `0`. Index: clang-tools-extra/trunk/test/clang-tidy/performance-implicit-cast-in-loop.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-implicit-cast-in-loop.cpp +++ clang-tools-extra/trunk/test/clang-tidy/performance-implicit-cast-in-loop.cpp @@ -1,161 +0,0 @@ -// RUN: %check_clang_tidy %s performance-implicit-cast-in-loop %t - -// ---------- Classes used in the tests ---------- - -// Iterator returning by value. -template -struct Iterator { - void operator++(); - T operator*(); - bool operator!=(const Iterator& other); -}; - -// Iterator returning by reference. -template -struct RefIterator { - void operator++(); - T& operator*(); - bool operator!=(const RefIterator& other); -}; - -// The template argument is an iterator type, and a view is an object you can -// run a for loop on. -template -struct View { - T begin(); - T end(); -}; - -// With this class, the implicit cast is a call to the (implicit) constructor of -// the class. -template -class ImplicitWrapper { - public: - // Implicit! - ImplicitWrapper(const T& t); -}; - -// With this class, the implicit cast is a call to the conversion operators of -// SimpleClass and ComplexClass. -template -class OperatorWrapper { - public: - explicit OperatorWrapper(const T& t); -}; - -struct SimpleClass { - int foo; - operator OperatorWrapper(); -}; - -// The materialize expression is not the same when the class has a destructor, -// so we make sure we cover that case too. -class ComplexClass { - public: - ComplexClass(); - ~ComplexClass(); - operator OperatorWrapper(); -}; - -typedef View> SimpleView; -typedef View> SimpleRefView; -typedef View> ComplexView; -typedef View> ComplexRefView; - -// ---------- The test themselves ---------- -// For each test we do, in the same order, const ref, non const ref, const -// value, non const value. - -void SimpleClassIterator() { - for (const SimpleClass& foo : SimpleView()) {} - // This line does not compile because a temporary cannot be assigned to a non - // const reference. - // for (SimpleClass& foo : SimpleView()) {} - for (const SimpleClass foo : SimpleView()) {} - for (SimpleClass foo : SimpleView()) {} -} - -void SimpleClassRefIterator() { - for (const SimpleClass& foo : SimpleRefView()) {} - for (SimpleClass& foo : SimpleRefView()) {} - for (const SimpleClass foo : SimpleRefView()) {} - for (SimpleClass foo : SimpleRefView()) {} -} - -void ComplexClassIterator() { - for (const ComplexClass& foo : ComplexView()) {} - // for (ComplexClass& foo : ComplexView()) {} - for (const ComplexClass foo : ComplexView()) {} - for (ComplexClass foo : ComplexView()) {} -} - -void ComplexClassRefIterator() { - for (const ComplexClass& foo : ComplexRefView()) {} - for (ComplexClass& foo : ComplexRefView()) {} - for (const ComplexClass foo : ComplexRefView()) {} - for (ComplexClass foo : ComplexRefView()) {} -} - -void ImplicitSimpleClassIterator() { - for (const ImplicitWrapper& foo : SimpleView()) {} - // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the loop variable 'foo' is different from the one returned by the iterator and generates an implicit cast; you can either change the type to the correct one ('const SimpleClass &' but 'const auto&' is always a valid option) or remove the reference to make it explicit that you are creating a new value [performance-implicit-cast-in-loop] - // for (ImplicitWrapper& foo : SimpleView()) {} - for (const ImplicitWrapper foo : SimpleView()) {} - for (ImplicitWrapperfoo : SimpleView()) {} -} - -void ImplicitSimpleClassRefIterator() { - for (const ImplicitWrapper& foo : SimpleRefView()) {} - // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} - // for (ImplicitWrapper& foo : SimpleRefView()) {} - for (const ImplicitWrapper foo : SimpleRefView()) {} - for (ImplicitWrapperfoo : SimpleRefView()) {} -} - -void ImplicitComplexClassIterator() { - for (const ImplicitWrapper& foo : ComplexView()) {} - // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} - // for (ImplicitWrapper& foo : ComplexView()) {} - for (const ImplicitWrapper foo : ComplexView()) {} - for (ImplicitWrapperfoo : ComplexView()) {} -} - -void ImplicitComplexClassRefIterator() { - for (const ImplicitWrapper& foo : ComplexRefView()) {} - // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} - // for (ImplicitWrapper& foo : ComplexRefView()) {} - for (const ImplicitWrapper foo : ComplexRefView()) {} - for (ImplicitWrapperfoo : ComplexRefView()) {} -} - -void OperatorSimpleClassIterator() { - for (const OperatorWrapper& foo : SimpleView()) {} - // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} - // for (OperatorWrapper& foo : SimpleView()) {} - for (const OperatorWrapper foo : SimpleView()) {} - for (OperatorWrapperfoo : SimpleView()) {} -} - -void OperatorSimpleClassRefIterator() { - for (const OperatorWrapper& foo : SimpleRefView()) {} - // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} - // for (OperatorWrapper& foo : SimpleRefView()) {} - for (const OperatorWrapper foo : SimpleRefView()) {} - for (OperatorWrapperfoo : SimpleRefView()) {} -} - -void OperatorComplexClassIterator() { - for (const OperatorWrapper& foo : ComplexView()) {} - // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} - // for (OperatorWrapper& foo : ComplexView()) {} - for (const OperatorWrapper foo : ComplexView()) {} - for (OperatorWrapperfoo : ComplexView()) {} -} - -void OperatorComplexClassRefIterator() { - for (const OperatorWrapper& foo : ComplexRefView()) {} - // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} - // for (OperatorWrapper& foo : ComplexRefView()) {} - for (const OperatorWrapper foo : ComplexRefView()) {} - for (OperatorWrapperfoo : ComplexRefView()) {} -} Index: clang-tools-extra/trunk/test/clang-tidy/performance-implicit-conversion-in-loop.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-implicit-conversion-in-loop.cpp +++ clang-tools-extra/trunk/test/clang-tidy/performance-implicit-conversion-in-loop.cpp @@ -0,0 +1,161 @@ +// RUN: %check_clang_tidy %s performance-implicit-conversion-in-loop %t + +// ---------- Classes used in the tests ---------- + +// Iterator returning by value. +template +struct Iterator { + void operator++(); + T operator*(); + bool operator!=(const Iterator& other); +}; + +// Iterator returning by reference. +template +struct RefIterator { + void operator++(); + T& operator*(); + bool operator!=(const RefIterator& other); +}; + +// The template argument is an iterator type, and a view is an object you can +// run a for loop on. +template +struct View { + T begin(); + T end(); +}; + +// With this class, the implicit conversion is a call to the (implicit) +// constructor of the class. +template +class ImplicitWrapper { + public: + // Implicit! + ImplicitWrapper(const T& t); +}; + +// With this class, the implicit conversion is a call to the conversion +// operators of SimpleClass and ComplexClass. +template +class OperatorWrapper { + public: + explicit OperatorWrapper(const T& t); +}; + +struct SimpleClass { + int foo; + operator OperatorWrapper(); +}; + +// The materialize expression is not the same when the class has a destructor, +// so we make sure we cover that case too. +class ComplexClass { + public: + ComplexClass(); + ~ComplexClass(); + operator OperatorWrapper(); +}; + +typedef View> SimpleView; +typedef View> SimpleRefView; +typedef View> ComplexView; +typedef View> ComplexRefView; + +// ---------- The test themselves ---------- +// For each test we do, in the same order, const ref, non const ref, const +// value, non const value. + +void SimpleClassIterator() { + for (const SimpleClass& foo : SimpleView()) {} + // This line does not compile because a temporary cannot be assigned to a non + // const reference. + // for (SimpleClass& foo : SimpleView()) {} + for (const SimpleClass foo : SimpleView()) {} + for (SimpleClass foo : SimpleView()) {} +} + +void SimpleClassRefIterator() { + for (const SimpleClass& foo : SimpleRefView()) {} + for (SimpleClass& foo : SimpleRefView()) {} + for (const SimpleClass foo : SimpleRefView()) {} + for (SimpleClass foo : SimpleRefView()) {} +} + +void ComplexClassIterator() { + for (const ComplexClass& foo : ComplexView()) {} + // for (ComplexClass& foo : ComplexView()) {} + for (const ComplexClass foo : ComplexView()) {} + for (ComplexClass foo : ComplexView()) {} +} + +void ComplexClassRefIterator() { + for (const ComplexClass& foo : ComplexRefView()) {} + for (ComplexClass& foo : ComplexRefView()) {} + for (const ComplexClass foo : ComplexRefView()) {} + for (ComplexClass foo : ComplexRefView()) {} +} + +void ImplicitSimpleClassIterator() { + for (const ImplicitWrapper& foo : SimpleView()) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the loop variable 'foo' is different from the one returned by the iterator and generates an implicit conversion; you can either change the type to the matching one ('const SimpleClass &' but 'const auto&' is always a valid option) or remove the reference to make it explicit that you are creating a new value [performance-implicit-conversion-in-loop] + // for (ImplicitWrapper& foo : SimpleView()) {} + for (const ImplicitWrapper foo : SimpleView()) {} + for (ImplicitWrapperfoo : SimpleView()) {} +} + +void ImplicitSimpleClassRefIterator() { + for (const ImplicitWrapper& foo : SimpleRefView()) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} + // for (ImplicitWrapper& foo : SimpleRefView()) {} + for (const ImplicitWrapper foo : SimpleRefView()) {} + for (ImplicitWrapperfoo : SimpleRefView()) {} +} + +void ImplicitComplexClassIterator() { + for (const ImplicitWrapper& foo : ComplexView()) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} + // for (ImplicitWrapper& foo : ComplexView()) {} + for (const ImplicitWrapper foo : ComplexView()) {} + for (ImplicitWrapperfoo : ComplexView()) {} +} + +void ImplicitComplexClassRefIterator() { + for (const ImplicitWrapper& foo : ComplexRefView()) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} + // for (ImplicitWrapper& foo : ComplexRefView()) {} + for (const ImplicitWrapper foo : ComplexRefView()) {} + for (ImplicitWrapperfoo : ComplexRefView()) {} +} + +void OperatorSimpleClassIterator() { + for (const OperatorWrapper& foo : SimpleView()) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} + // for (OperatorWrapper& foo : SimpleView()) {} + for (const OperatorWrapper foo : SimpleView()) {} + for (OperatorWrapperfoo : SimpleView()) {} +} + +void OperatorSimpleClassRefIterator() { + for (const OperatorWrapper& foo : SimpleRefView()) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const SimpleClass &'.*}} + // for (OperatorWrapper& foo : SimpleRefView()) {} + for (const OperatorWrapper foo : SimpleRefView()) {} + for (OperatorWrapperfoo : SimpleRefView()) {} +} + +void OperatorComplexClassIterator() { + for (const OperatorWrapper& foo : ComplexView()) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} + // for (OperatorWrapper& foo : ComplexView()) {} + for (const OperatorWrapper foo : ComplexView()) {} + for (OperatorWrapperfoo : ComplexView()) {} +} + +void OperatorComplexClassRefIterator() { + for (const OperatorWrapper& foo : ComplexRefView()) {} + // CHECK-MESSAGES: [[@LINE-1]]:{{[0-9]*}}: warning: the type of the{{.*'const ComplexClass &'.*}} + // for (OperatorWrapper& foo : ComplexRefView()) {} + for (const OperatorWrapper foo : ComplexRefView()) {} + for (OperatorWrapperfoo : ComplexRefView()) {} +} Index: clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-allow-conditional-casts.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-allow-conditional-casts.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-allow-conditional-casts.cpp @@ -1,66 +0,0 @@ -// RUN: %check_clang_tidy %s readability-implicit-bool-cast %t \ -// RUN: -config='{CheckOptions: \ -// RUN: [{key: readability-implicit-bool-cast.AllowConditionalIntegerCasts, value: 1}, \ -// RUN: {key: readability-implicit-bool-cast.AllowConditionalPointerCasts, value: 1}]}' \ -// RUN: -- -std=c++11 - -template -void functionTaking(T); - -int functionReturningInt(); -int* functionReturningPointer(); - -struct Struct { - int member; -}; - - -void regularImplicitCastIntegerToBoolIsNotIgnored() { - int integer = 0; - functionTaking(integer); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool [readability-implicit-bool-cast] - // CHECK-FIXES: functionTaking(integer != 0); -} - -void implicitCastIntegerToBoolInConditionalsIsAllowed() { - if (functionReturningInt()) {} - if (!functionReturningInt()) {} - if (functionReturningInt() && functionReturningPointer()) {} - if (!functionReturningInt() && !functionReturningPointer()) {} - for (; functionReturningInt(); ) {} - for (; functionReturningPointer(); ) {} - for (; functionReturningInt() && !functionReturningPointer() || (!functionReturningInt() && functionReturningPointer()); ) {} - while (functionReturningInt()) {} - while (functionReturningPointer()) {} - while (functionReturningInt() && !functionReturningPointer() || (!functionReturningInt() && functionReturningPointer())) {} - int value1 = functionReturningInt() ? 1 : 2; - int value2 = !functionReturningInt() ? 1 : 2; - int value3 = (functionReturningInt() && functionReturningPointer() || !functionReturningInt()) ? 1 : 2; - int value4 = functionReturningInt() ?: value3; - int *p1 = functionReturningPointer() ?: &value3; -} - -void regularImplicitCastPointerToBoolIsNotIgnored() { - int* pointer = nullptr; - functionTaking(pointer); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int *' -> bool - // CHECK-FIXES: functionTaking(pointer != nullptr); - - int Struct::* memberPointer = &Struct::member; - functionTaking(memberPointer); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int Struct::*' -> bool - // CHECK-FIXES: functionTaking(memberPointer != nullptr); -} - -void implicitCastPointerToBoolInConditionalsIsAllowed() { - if (functionReturningPointer()) {} - if (not functionReturningPointer()) {} - int value1 = functionReturningPointer() ? 1 : 2; - int value2 = (not functionReturningPointer()) ? 1 : 2; - - int Struct::* memberPointer = &Struct::member; - if (memberPointer) {} - if (memberPointer) {} - int value3 = memberPointer ? 1 : 2; - int value4 = (not memberPointer) ? 1 : 2; -} Index: clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-cxx98.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-cxx98.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast-cxx98.cpp @@ -1,45 +0,0 @@ -// RUN: %check_clang_tidy %s readability-implicit-bool-cast %t -- -- -std=c++98 - -// We need NULL macro, but some buildbots don't like including header -// This is a portable way of getting it to work -#undef NULL -#define NULL 0L - -template -void functionTaking(T); - -struct Struct { - int member; -}; - -void useOldNullMacroInReplacements() { - int* pointer = NULL; - functionTaking(pointer); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int *' -> bool [readability-implicit-bool-cast] - // CHECK-FIXES: functionTaking(pointer != 0); - - int Struct::* memberPointer = NULL; - functionTaking(!memberPointer); - // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit cast 'int Struct::*' -> bool - // CHECK-FIXES: functionTaking(memberPointer == 0); -} - -void fixFalseLiteralConvertingToNullPointer() { - functionTaking(false); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast bool -> 'int *' - // CHECK-FIXES: functionTaking(0); - - int* pointer = NULL; - if (pointer == false) {} - // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicit cast bool -> 'int *' - // CHECK-FIXES: if (pointer == 0) {} - - functionTaking(false); - // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit cast bool -> 'int Struct::*' - // CHECK-FIXES: functionTaking(0); - - int Struct::* memberPointer = NULL; - if (memberPointer != false) {} - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast bool -> 'int Struct::*' - // CHECK-FIXES: if (memberPointer != 0) {} -} Index: clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-cast.cpp @@ -1,439 +0,0 @@ -// RUN: %check_clang_tidy %s readability-implicit-bool-cast %t - -// We need NULL macro, but some buildbots don't like including header -// This is a portable way of getting it to work -#undef NULL -#define NULL 0L - -template -void functionTaking(T); - -struct Struct { - int member; -}; - - -////////// Implicit cast from bool. - -void implicitCastFromBoolSimpleCases() { - bool boolean = true; - - functionTaking(boolean); - - functionTaking(boolean); - // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit cast bool -> 'int' [readability-implicit-bool-cast] - // CHECK-FIXES: functionTaking(static_cast(boolean)); - - functionTaking(boolean); - // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit cast bool -> 'unsigned long' - // CHECK-FIXES: functionTaking(static_cast(boolean)); - - functionTaking(boolean); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast bool -> 'char' - // CHECK-FIXES: functionTaking(static_cast(boolean)); - - functionTaking(boolean); - // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit cast bool -> 'float' - // CHECK-FIXES: functionTaking(static_cast(boolean)); - - functionTaking(boolean); - // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: implicit cast bool -> 'double' - // CHECK-FIXES: functionTaking(static_cast(boolean)); -} - -float implicitCastFromBoolInReturnValue() { - bool boolean = false; - return boolean; - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit cast bool -> 'float' - // CHECK-FIXES: return static_cast(boolean); -} - -void implicitCastFromBoolInSingleBoolExpressions(bool b1, bool b2) { - bool boolean = true; - boolean = b1 ^ b2; - boolean = b1 && b2; - boolean |= !b1 || !b2; - boolean &= b1; - boolean = b1 == true; - boolean = b2 != false; - - int integer = boolean - 3; - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit cast bool -> 'int' - // CHECK-FIXES: int integer = static_cast(boolean) - 3; - - float floating = boolean / 0.3f; - // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit cast bool -> 'float' - // CHECK-FIXES: float floating = static_cast(boolean) / 0.3f; - - char character = boolean; - // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit cast bool -> 'char' - // CHECK-FIXES: char character = static_cast(boolean); -} - -void implicitCastFromBoollInComplexBoolExpressions() { - bool boolean = true; - bool anotherBoolean = false; - - int integer = boolean && anotherBoolean; - // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit cast bool -> 'int' - // CHECK-FIXES: int integer = static_cast(boolean && anotherBoolean); - - unsigned long unsignedLong = (! boolean) + 4ul; - // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit cast bool -> 'unsigned long' - // CHECK-FIXES: unsigned long unsignedLong = static_cast(! boolean) + 4ul; - - float floating = (boolean || anotherBoolean) * 0.3f; - // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit cast bool -> 'float' - // CHECK-FIXES: float floating = static_cast(boolean || anotherBoolean) * 0.3f; - - double doubleFloating = (boolean && (anotherBoolean || boolean)) * 0.3; - // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: implicit cast bool -> 'double' - // CHECK-FIXES: double doubleFloating = static_cast(boolean && (anotherBoolean || boolean)) * 0.3; -} - -void implicitCastFromBoolLiterals() { - functionTaking(true); - // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit cast bool -> 'int' - // CHECK-FIXES: functionTaking(1); - - functionTaking(false); - // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit cast bool -> 'unsigned long' - // CHECK-FIXES: functionTaking(0u); - - functionTaking(true); - // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: implicit cast bool -> 'signed char' - // CHECK-FIXES: functionTaking(1); - - functionTaking(false); - // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit cast bool -> 'float' - // CHECK-FIXES: functionTaking(0.0f); - - functionTaking(true); - // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: implicit cast bool -> 'double' - // CHECK-FIXES: functionTaking(1.0); -} - -void implicitCastFromBoolInComparisons() { - bool boolean = true; - int integer = 0; - - functionTaking(boolean == integer); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast bool -> 'int' - // CHECK-FIXES: functionTaking(static_cast(boolean) == integer); - - functionTaking(integer != boolean); - // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: implicit cast bool -> 'int' - // CHECK-FIXES: functionTaking(integer != static_cast(boolean)); -} - -void ignoreBoolComparisons() { - bool boolean = true; - bool anotherBoolean = false; - - functionTaking(boolean == anotherBoolean); - functionTaking(boolean != anotherBoolean); -} - -void ignoreExplicitCastsFromBool() { - bool boolean = true; - - int integer = static_cast(boolean) + 3; - float floating = static_cast(boolean) * 0.3f; - char character = static_cast(boolean); -} - -void ignoreImplicitCastFromBoolInMacroExpansions() { - bool boolean = true; - - #define CAST_FROM_BOOL_IN_MACRO_BODY boolean + 3 - int integerFromMacroBody = CAST_FROM_BOOL_IN_MACRO_BODY; - - #define CAST_FROM_BOOL_IN_MACRO_ARGUMENT(x) x + 3 - int integerFromMacroArgument = CAST_FROM_BOOL_IN_MACRO_ARGUMENT(boolean); -} - -namespace ignoreImplicitCastFromBoolInTemplateInstantiations { - -template -void templateFunction() { - bool boolean = true; - T uknownType = boolean + 3; -} - -void useOfTemplateFunction() { - templateFunction(); -} - -} // namespace ignoreImplicitCastFromBoolInTemplateInstantiations - -////////// Implicit cast to bool. - -void implicitCastToBoolSimpleCases() { - int integer = 10; - functionTaking(integer); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool - // CHECK-FIXES: functionTaking(integer != 0); - - unsigned long unsignedLong = 10; - functionTaking(unsignedLong); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'unsigned long' -> bool - // CHECK-FIXES: functionTaking(unsignedLong != 0u); - - float floating = 0.0f; - functionTaking(floating); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'float' -> bool - // CHECK-FIXES: functionTaking(floating != 0.0f); - - double doubleFloating = 1.0f; - functionTaking(doubleFloating); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'double' -> bool - // CHECK-FIXES: functionTaking(doubleFloating != 0.0); - - signed char character = 'a'; - functionTaking(character); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'signed char' -> bool - // CHECK-FIXES: functionTaking(character != 0); - - int* pointer = nullptr; - functionTaking(pointer); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int *' -> bool - // CHECK-FIXES: functionTaking(pointer != nullptr); - - auto pointerToMember = &Struct::member; - functionTaking(pointerToMember); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int Struct::*' -> bool - // CHECK-FIXES: functionTaking(pointerToMember != nullptr); -} - -void implicitCastToBoolInSingleExpressions() { - int integer = 10; - bool boolComingFromInt = integer; - // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: implicit cast 'int' -> bool - // CHECK-FIXES: bool boolComingFromInt = integer != 0; - - float floating = 10.0f; - bool boolComingFromFloat = floating; - // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: implicit cast 'float' -> bool - // CHECK-FIXES: bool boolComingFromFloat = floating != 0.0f; - - signed char character = 'a'; - bool boolComingFromChar = character; - // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: implicit cast 'signed char' -> bool - // CHECK-FIXES: bool boolComingFromChar = character != 0; - - int* pointer = nullptr; - bool boolComingFromPointer = pointer; - // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit cast 'int *' -> bool - // CHECK-FIXES: bool boolComingFromPointer = pointer != nullptr; -} - -void implicitCastToBoolInComplexExpressions() { - bool boolean = true; - - int integer = 10; - int anotherInteger = 20; - bool boolComingFromInteger = integer + anotherInteger; - // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit cast 'int' -> bool - // CHECK-FIXES: bool boolComingFromInteger = (integer + anotherInteger) != 0; - - float floating = 0.2f; - bool boolComingFromFloating = floating - 0.3f || boolean; - // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit cast 'float' -> bool - // CHECK-FIXES: bool boolComingFromFloating = ((floating - 0.3f) != 0.0f) || boolean; - - double doubleFloating = 0.3; - bool boolComingFromDoubleFloating = (doubleFloating - 0.4) && boolean; - // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: implicit cast 'double' -> bool - // CHECK-FIXES: bool boolComingFromDoubleFloating = ((doubleFloating - 0.4) != 0.0) && boolean; -} - -void implicitCastInNegationExpressions() { - int integer = 10; - bool boolComingFromNegatedInt = !integer; - // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: implicit cast 'int' -> bool - // CHECK-FIXES: bool boolComingFromNegatedInt = integer == 0; - - float floating = 10.0f; - bool boolComingFromNegatedFloat = ! floating; - // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: implicit cast 'float' -> bool - // CHECK-FIXES: bool boolComingFromNegatedFloat = floating == 0.0f; - - signed char character = 'a'; - bool boolComingFromNegatedChar = (! character); - // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: implicit cast 'signed char' -> bool - // CHECK-FIXES: bool boolComingFromNegatedChar = (character == 0); - - int* pointer = nullptr; - bool boolComingFromNegatedPointer = not pointer; - // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: implicit cast 'int *' -> bool - // CHECK-FIXES: bool boolComingFromNegatedPointer = pointer == nullptr; -} - -void implicitCastToBoolInControlStatements() { - int integer = 10; - if (integer) {} - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicit cast 'int' -> bool - // CHECK-FIXES: if (integer != 0) {} - - long int longInteger = 0.2f; - for (;longInteger;) {} - // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: implicit cast 'long' -> bool - // CHECK-FIXES: for (;longInteger != 0;) {} - - float floating = 0.3f; - while (floating) {} - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit cast 'float' -> bool - // CHECK-FIXES: while (floating != 0.0f) {} - - double doubleFloating = 0.4; - do {} while (doubleFloating); - // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: implicit cast 'double' -> bool - // CHECK-FIXES: do {} while (doubleFloating != 0.0); -} - -bool implicitCastToBoolInReturnValue() { - float floating = 1.0f; - return floating; - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit cast 'float' -> bool - // CHECK-FIXES: return floating != 0.0f; -} - -void implicitCastToBoolFromLiterals() { - functionTaking(0); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool - // CHECK-FIXES: functionTaking(false); - - functionTaking(1); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool - // CHECK-FIXES: functionTaking(true); - - functionTaking(2ul); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'unsigned long' -> bool - // CHECK-FIXES: functionTaking(true); - - - functionTaking(0.0f); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'float' -> bool - // CHECK-FIXES: functionTaking(false); - - functionTaking(1.0f); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'float' -> bool - // CHECK-FIXES: functionTaking(true); - - functionTaking(2.0); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'double' -> bool - // CHECK-FIXES: functionTaking(true); - - - functionTaking('\0'); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'char' -> bool - // CHECK-FIXES: functionTaking(false); - - functionTaking('a'); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'char' -> bool - // CHECK-FIXES: functionTaking(true); - - - functionTaking(""); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'const char *' -> bool - // CHECK-FIXES: functionTaking(true); - - functionTaking("abc"); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'const char *' -> bool - // CHECK-FIXES: functionTaking(true); - - functionTaking(NULL); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'long' -> bool - // CHECK-FIXES: functionTaking(false); -} - -void implicitCastToBoolFromUnaryMinusAndZeroLiterals() { - functionTaking(-0); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool - // CHECK-FIXES: functionTaking((-0) != 0); - - functionTaking(-0.0f); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'float' -> bool - // CHECK-FIXES: functionTaking((-0.0f) != 0.0f); - - functionTaking(-0.0); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'double' -> bool - // CHECK-FIXES: functionTaking((-0.0) != 0.0); -} - -void implicitCastToBoolInWithOverloadedOperators() { - struct UserStruct { - int operator()(int x) { return x; } - int operator+(int y) { return y; } - }; - - UserStruct s; - - functionTaking(s(0)); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool - // CHECK-FIXES: functionTaking(s(0) != 0); - - functionTaking(s + 2); - // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit cast 'int' -> bool - // CHECK-FIXES: functionTaking((s + 2) != 0); -} - -int functionReturningInt(); -int* functionReturningPointer(); - -void ignoreImplicitCastToBoolWhenDeclaringVariableInControlStatements() { - if (int integer = functionReturningInt()) {} - - while (int* pointer = functionReturningPointer()) {} -} - -void ignoreExplicitCastsToBool() { - int integer = 10; - bool boolComingFromInt = static_cast(integer); - - float floating = 10.0f; - bool boolComingFromFloat = static_cast(floating); - - char character = 'a'; - bool boolComingFromChar = static_cast(character); - - int* pointer = nullptr; - bool booleanComingFromPointer = static_cast(pointer); -} - -void ignoreImplicitCastToBoolInMacroExpansions() { - int integer = 3; - - #define CAST_TO_BOOL_IN_MACRO_BODY integer && false - bool boolFromMacroBody = CAST_TO_BOOL_IN_MACRO_BODY; - - #define CAST_TO_BOOL_IN_MACRO_ARGUMENT(x) x || true - bool boolFromMacroArgument = CAST_TO_BOOL_IN_MACRO_ARGUMENT(integer); -} - -namespace ignoreImplicitCastToBoolInTemplateInstantiations { - -template -void templateFunction() { - T unknownType = 0; - bool boolean = unknownType; -} - -void useOfTemplateFunction() { - templateFunction(); -} - -} // namespace ignoreImplicitCastToBoolInTemplateInstantiations - -namespace ignoreUserDefinedConversionOperator { - -struct StructWithUserConversion { - operator bool(); -}; - -void useOfUserConversion() { - StructWithUserConversion structure; - functionTaking(structure); -} - -} // namespace ignoreUserDefinedConversionOperator Index: clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-conversion-allow-in-conditions.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-conversion-allow-in-conditions.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-conversion-allow-in-conditions.cpp @@ -0,0 +1,66 @@ +// RUN: %check_clang_tidy %s readability-implicit-bool-conversion %t \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: readability-implicit-bool-conversion.AllowIntegerConditions, value: 1}, \ +// RUN: {key: readability-implicit-bool-conversion.AllowPointerConditions, value: 1}]}' \ +// RUN: -- -std=c++11 + +template +void functionTaking(T); + +int functionReturningInt(); +int* functionReturningPointer(); + +struct Struct { + int member; +}; + + +void regularImplicitConversionIntegerToBoolIsNotIgnored() { + int integer = 0; + functionTaking(integer); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> bool [readability-implicit-bool-conversion] + // CHECK-FIXES: functionTaking(integer != 0); +} + +void implicitConversionIntegerToBoolInConditionalsIsAllowed() { + if (functionReturningInt()) {} + if (!functionReturningInt()) {} + if (functionReturningInt() && functionReturningPointer()) {} + if (!functionReturningInt() && !functionReturningPointer()) {} + for (; functionReturningInt(); ) {} + for (; functionReturningPointer(); ) {} + for (; functionReturningInt() && !functionReturningPointer() || (!functionReturningInt() && functionReturningPointer()); ) {} + while (functionReturningInt()) {} + while (functionReturningPointer()) {} + while (functionReturningInt() && !functionReturningPointer() || (!functionReturningInt() && functionReturningPointer())) {} + int value1 = functionReturningInt() ? 1 : 2; + int value2 = !functionReturningInt() ? 1 : 2; + int value3 = (functionReturningInt() && functionReturningPointer() || !functionReturningInt()) ? 1 : 2; + int value4 = functionReturningInt() ?: value3; + int *p1 = functionReturningPointer() ?: &value3; +} + +void regularImplicitConversionPointerToBoolIsNotIgnored() { + int* pointer = nullptr; + functionTaking(pointer); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> bool + // CHECK-FIXES: functionTaking(pointer != nullptr); + + int Struct::* memberPointer = &Struct::member; + functionTaking(memberPointer); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int Struct::*' -> bool + // CHECK-FIXES: functionTaking(memberPointer != nullptr); +} + +void implicitConversionPointerToBoolInConditionalsIsAllowed() { + if (functionReturningPointer()) {} + if (not functionReturningPointer()) {} + int value1 = functionReturningPointer() ? 1 : 2; + int value2 = (not functionReturningPointer()) ? 1 : 2; + + int Struct::* memberPointer = &Struct::member; + if (memberPointer) {} + if (memberPointer) {} + int value3 = memberPointer ? 1 : 2; + int value4 = (not memberPointer) ? 1 : 2; +} Index: clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-conversion-cxx98.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-conversion-cxx98.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-conversion-cxx98.cpp @@ -0,0 +1,45 @@ +// RUN: %check_clang_tidy %s readability-implicit-bool-conversion %t -- -- -std=c++98 + +// We need NULL macro, but some buildbots don't like including header +// This is a portable way of getting it to work +#undef NULL +#define NULL 0L + +template +void functionTaking(T); + +struct Struct { + int member; +}; + +void useOldNullMacroInReplacements() { + int* pointer = NULL; + functionTaking(pointer); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> bool [readability-implicit-bool-conversion] + // CHECK-FIXES: functionTaking(pointer != 0); + + int Struct::* memberPointer = NULL; + functionTaking(!memberPointer); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion 'int Struct::*' -> bool + // CHECK-FIXES: functionTaking(memberPointer == 0); +} + +void fixFalseLiteralConvertingToNullPointer() { + functionTaking(false); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion bool -> 'int *' + // CHECK-FIXES: functionTaking(0); + + int* pointer = NULL; + if (pointer == false) {} + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicit conversion bool -> 'int *' + // CHECK-FIXES: if (pointer == 0) {} + + functionTaking(false); + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion bool -> 'int Struct::*' + // CHECK-FIXES: functionTaking(0); + + int Struct::* memberPointer = NULL; + if (memberPointer != false) {} + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion bool -> 'int Struct::*' + // CHECK-FIXES: if (memberPointer != 0) {} +} Index: clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-conversion.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-conversion.cpp +++ clang-tools-extra/trunk/test/clang-tidy/readability-implicit-bool-conversion.cpp @@ -0,0 +1,439 @@ +// RUN: %check_clang_tidy %s readability-implicit-bool-conversion %t + +// We need NULL macro, but some buildbots don't like including header +// This is a portable way of getting it to work +#undef NULL +#define NULL 0L + +template +void functionTaking(T); + +struct Struct { + int member; +}; + + +////////// Implicit conversion from bool. + +void implicitConversionFromBoolSimpleCases() { + bool boolean = true; + + functionTaking(boolean); + + functionTaking(boolean); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit conversion bool -> 'int' [readability-implicit-bool-conversion] + // CHECK-FIXES: functionTaking(static_cast(boolean)); + + functionTaking(boolean); + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion bool -> 'unsigned long' + // CHECK-FIXES: functionTaking(static_cast(boolean)); + + functionTaking(boolean); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion bool -> 'char' + // CHECK-FIXES: functionTaking(static_cast(boolean)); + + functionTaking(boolean); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion bool -> 'float' + // CHECK-FIXES: functionTaking(static_cast(boolean)); + + functionTaking(boolean); + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: implicit conversion bool -> 'double' + // CHECK-FIXES: functionTaking(static_cast(boolean)); +} + +float implicitConversionFromBoolInReturnValue() { + bool boolean = false; + return boolean; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion bool -> 'float' + // CHECK-FIXES: return static_cast(boolean); +} + +void implicitConversionFromBoolInSingleBoolExpressions(bool b1, bool b2) { + bool boolean = true; + boolean = b1 ^ b2; + boolean = b1 && b2; + boolean |= !b1 || !b2; + boolean &= b1; + boolean = b1 == true; + boolean = b2 != false; + + int integer = boolean - 3; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit conversion bool -> 'int' + // CHECK-FIXES: int integer = static_cast(boolean) - 3; + + float floating = boolean / 0.3f; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion bool -> 'float' + // CHECK-FIXES: float floating = static_cast(boolean) / 0.3f; + + char character = boolean; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion bool -> 'char' + // CHECK-FIXES: char character = static_cast(boolean); +} + +void implicitConversionFromBoollInComplexBoolExpressions() { + bool boolean = true; + bool anotherBoolean = false; + + int integer = boolean && anotherBoolean; + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicit conversion bool -> 'int' + // CHECK-FIXES: int integer = static_cast(boolean && anotherBoolean); + + unsigned long unsignedLong = (! boolean) + 4ul; + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit conversion bool -> 'unsigned long' + // CHECK-FIXES: unsigned long unsignedLong = static_cast(! boolean) + 4ul; + + float floating = (boolean || anotherBoolean) * 0.3f; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: implicit conversion bool -> 'float' + // CHECK-FIXES: float floating = static_cast(boolean || anotherBoolean) * 0.3f; + + double doubleFloating = (boolean && (anotherBoolean || boolean)) * 0.3; + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: implicit conversion bool -> 'double' + // CHECK-FIXES: double doubleFloating = static_cast(boolean && (anotherBoolean || boolean)) * 0.3; +} + +void implicitConversionFromBoolLiterals() { + functionTaking(true); + // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: implicit conversion bool -> 'int' + // CHECK-FIXES: functionTaking(1); + + functionTaking(false); + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion bool -> 'unsigned long' + // CHECK-FIXES: functionTaking(0u); + + functionTaking(true); + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: implicit conversion bool -> 'signed char' + // CHECK-FIXES: functionTaking(1); + + functionTaking(false); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicit conversion bool -> 'float' + // CHECK-FIXES: functionTaking(0.0f); + + functionTaking(true); + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: implicit conversion bool -> 'double' + // CHECK-FIXES: functionTaking(1.0); +} + +void implicitConversionFromBoolInComparisons() { + bool boolean = true; + int integer = 0; + + functionTaking(boolean == integer); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion bool -> 'int' + // CHECK-FIXES: functionTaking(static_cast(boolean) == integer); + + functionTaking(integer != boolean); + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: implicit conversion bool -> 'int' + // CHECK-FIXES: functionTaking(integer != static_cast(boolean)); +} + +void ignoreBoolComparisons() { + bool boolean = true; + bool anotherBoolean = false; + + functionTaking(boolean == anotherBoolean); + functionTaking(boolean != anotherBoolean); +} + +void ignoreExplicitCastsFromBool() { + bool boolean = true; + + int integer = static_cast(boolean) + 3; + float floating = static_cast(boolean) * 0.3f; + char character = static_cast(boolean); +} + +void ignoreImplicitConversionFromBoolInMacroExpansions() { + bool boolean = true; + + #define CAST_FROM_BOOL_IN_MACRO_BODY boolean + 3 + int integerFromMacroBody = CAST_FROM_BOOL_IN_MACRO_BODY; + + #define CAST_FROM_BOOL_IN_MACRO_ARGUMENT(x) x + 3 + int integerFromMacroArgument = CAST_FROM_BOOL_IN_MACRO_ARGUMENT(boolean); +} + +namespace ignoreImplicitConversionFromBoolInTemplateInstantiations { + +template +void templateFunction() { + bool boolean = true; + T uknownType = boolean + 3; +} + +void useOfTemplateFunction() { + templateFunction(); +} + +} // namespace ignoreImplicitConversionFromBoolInTemplateInstantiations + +////////// Implicit conversions to bool. + +void implicitConversionToBoolSimpleCases() { + int integer = 10; + functionTaking(integer); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> bool + // CHECK-FIXES: functionTaking(integer != 0); + + unsigned long unsignedLong = 10; + functionTaking(unsignedLong); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned long' -> bool + // CHECK-FIXES: functionTaking(unsignedLong != 0u); + + float floating = 0.0f; + functionTaking(floating); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> bool + // CHECK-FIXES: functionTaking(floating != 0.0f); + + double doubleFloating = 1.0f; + functionTaking(doubleFloating); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'double' -> bool + // CHECK-FIXES: functionTaking(doubleFloating != 0.0); + + signed char character = 'a'; + functionTaking(character); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'signed char' -> bool + // CHECK-FIXES: functionTaking(character != 0); + + int* pointer = nullptr; + functionTaking(pointer); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int *' -> bool + // CHECK-FIXES: functionTaking(pointer != nullptr); + + auto pointerToMember = &Struct::member; + functionTaking(pointerToMember); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int Struct::*' -> bool + // CHECK-FIXES: functionTaking(pointerToMember != nullptr); +} + +void implicitConversionToBoolInSingleExpressions() { + int integer = 10; + bool boolComingFromInt = integer; + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: implicit conversion 'int' -> bool + // CHECK-FIXES: bool boolComingFromInt = integer != 0; + + float floating = 10.0f; + bool boolComingFromFloat = floating; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: implicit conversion 'float' -> bool + // CHECK-FIXES: bool boolComingFromFloat = floating != 0.0f; + + signed char character = 'a'; + bool boolComingFromChar = character; + // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: implicit conversion 'signed char' -> bool + // CHECK-FIXES: bool boolComingFromChar = character != 0; + + int* pointer = nullptr; + bool boolComingFromPointer = pointer; + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit conversion 'int *' -> bool + // CHECK-FIXES: bool boolComingFromPointer = pointer != nullptr; +} + +void implicitConversionToBoolInComplexExpressions() { + bool boolean = true; + + int integer = 10; + int anotherInteger = 20; + bool boolComingFromInteger = integer + anotherInteger; + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: implicit conversion 'int' -> bool + // CHECK-FIXES: bool boolComingFromInteger = (integer + anotherInteger) != 0; + + float floating = 0.2f; + bool boolComingFromFloating = floating - 0.3f || boolean; + // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion 'float' -> bool + // CHECK-FIXES: bool boolComingFromFloating = ((floating - 0.3f) != 0.0f) || boolean; + + double doubleFloating = 0.3; + bool boolComingFromDoubleFloating = (doubleFloating - 0.4) && boolean; + // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: implicit conversion 'double' -> bool + // CHECK-FIXES: bool boolComingFromDoubleFloating = ((doubleFloating - 0.4) != 0.0) && boolean; +} + +void implicitConversionInNegationExpressions() { + int integer = 10; + bool boolComingFromNegatedInt = !integer; + // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: implicit conversion 'int' -> bool + // CHECK-FIXES: bool boolComingFromNegatedInt = integer == 0; + + float floating = 10.0f; + bool boolComingFromNegatedFloat = ! floating; + // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: implicit conversion 'float' -> bool + // CHECK-FIXES: bool boolComingFromNegatedFloat = floating == 0.0f; + + signed char character = 'a'; + bool boolComingFromNegatedChar = (! character); + // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: implicit conversion 'signed char' -> bool + // CHECK-FIXES: bool boolComingFromNegatedChar = (character == 0); + + int* pointer = nullptr; + bool boolComingFromNegatedPointer = not pointer; + // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: implicit conversion 'int *' -> bool + // CHECK-FIXES: bool boolComingFromNegatedPointer = pointer == nullptr; +} + +void implicitConversionToBoolInControlStatements() { + int integer = 10; + if (integer) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicit conversion 'int' -> bool + // CHECK-FIXES: if (integer != 0) {} + + long int longInteger = 0.2f; + for (;longInteger;) {} + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: implicit conversion 'long' -> bool + // CHECK-FIXES: for (;longInteger != 0;) {} + + float floating = 0.3f; + while (floating) {} + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion 'float' -> bool + // CHECK-FIXES: while (floating != 0.0f) {} + + double doubleFloating = 0.4; + do {} while (doubleFloating); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: implicit conversion 'double' -> bool + // CHECK-FIXES: do {} while (doubleFloating != 0.0); +} + +bool implicitConversionToBoolInReturnValue() { + float floating = 1.0f; + return floating; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicit conversion 'float' -> bool + // CHECK-FIXES: return floating != 0.0f; +} + +void implicitConversionToBoolFromLiterals() { + functionTaking(0); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> bool + // CHECK-FIXES: functionTaking(false); + + functionTaking(1); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> bool + // CHECK-FIXES: functionTaking(true); + + functionTaking(2ul); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned long' -> bool + // CHECK-FIXES: functionTaking(true); + + + functionTaking(0.0f); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> bool + // CHECK-FIXES: functionTaking(false); + + functionTaking(1.0f); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> bool + // CHECK-FIXES: functionTaking(true); + + functionTaking(2.0); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'double' -> bool + // CHECK-FIXES: functionTaking(true); + + + functionTaking('\0'); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'char' -> bool + // CHECK-FIXES: functionTaking(false); + + functionTaking('a'); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'char' -> bool + // CHECK-FIXES: functionTaking(true); + + + functionTaking(""); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'const char *' -> bool + // CHECK-FIXES: functionTaking(true); + + functionTaking("abc"); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'const char *' -> bool + // CHECK-FIXES: functionTaking(true); + + functionTaking(NULL); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'long' -> bool + // CHECK-FIXES: functionTaking(false); +} + +void implicitConversionToBoolFromUnaryMinusAndZeroLiterals() { + functionTaking(-0); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> bool + // CHECK-FIXES: functionTaking((-0) != 0); + + functionTaking(-0.0f); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> bool + // CHECK-FIXES: functionTaking((-0.0f) != 0.0f); + + functionTaking(-0.0); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'double' -> bool + // CHECK-FIXES: functionTaking((-0.0) != 0.0); +} + +void implicitConversionToBoolInWithOverloadedOperators() { + struct UserStruct { + int operator()(int x) { return x; } + int operator+(int y) { return y; } + }; + + UserStruct s; + + functionTaking(s(0)); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> bool + // CHECK-FIXES: functionTaking(s(0) != 0); + + functionTaking(s + 2); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'int' -> bool + // CHECK-FIXES: functionTaking((s + 2) != 0); +} + +int functionReturningInt(); +int* functionReturningPointer(); + +void ignoreImplicitConversionToBoolWhenDeclaringVariableInControlStatements() { + if (int integer = functionReturningInt()) {} + + while (int* pointer = functionReturningPointer()) {} +} + +void ignoreExplicitCastsToBool() { + int integer = 10; + bool boolComingFromInt = static_cast(integer); + + float floating = 10.0f; + bool boolComingFromFloat = static_cast(floating); + + char character = 'a'; + bool boolComingFromChar = static_cast(character); + + int* pointer = nullptr; + bool booleanComingFromPointer = static_cast(pointer); +} + +void ignoreImplicitConversionToBoolInMacroExpansions() { + int integer = 3; + + #define CAST_TO_BOOL_IN_MACRO_BODY integer && false + bool boolFromMacroBody = CAST_TO_BOOL_IN_MACRO_BODY; + + #define CAST_TO_BOOL_IN_MACRO_ARGUMENT(x) x || true + bool boolFromMacroArgument = CAST_TO_BOOL_IN_MACRO_ARGUMENT(integer); +} + +namespace ignoreImplicitConversionToBoolInTemplateInstantiations { + +template +void templateFunction() { + T unknownType = 0; + bool boolean = unknownType; +} + +void useOfTemplateFunction() { + templateFunction(); +} + +} // namespace ignoreImplicitConversionToBoolInTemplateInstantiations + +namespace ignoreUserDefinedConversionOperator { + +struct StructWithUserConversion { + operator bool(); +}; + +void useOfUserConversion() { + StructWithUserConversion structure; + functionTaking(structure); +} + +} // namespace ignoreUserDefinedConversionOperator