From a68f40374973e5ea579333e61f6ebbf425c78f09 Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Mon, 28 Dec 2015 13:06:58 +0000 Subject: [PATCH] Refactor: Simplify boolean conditional return statements in lib/StaticAnalyzer/Checkers Summary: Use clang-tidy to simplify boolean conditional return values Reviewers: dcoughlin, krememek Subscribers: krememek, cfe-commits Patch by Richard Thomson! Differential Revision: http://reviews.llvm.org/D10021 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@256491 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Checkers/BasicObjCFoundationChecks.cpp | 11 +++-------- .../Checkers/CStringChecker.cpp | 5 +---- .../Checkers/CastSizeChecker.cpp | 5 +---- .../Checkers/DirectIvarAssignment.cpp | 13 ++++++------- .../Checkers/GenericTaintChecker.cpp | 19 +++++-------------- .../Checkers/IdenticalExprChecker.cpp | 4 +--- .../Checkers/MacOSKeychainAPIChecker.cpp | 14 +++----------- lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 9 +++------ .../Checkers/ObjCContainersASTChecker.cpp | 5 ++--- .../Checkers/ObjCSelfInitChecker.cpp | 5 +---- .../Checkers/UnreachableCodeChecker.cpp | 9 +++------ 11 files changed, 29 insertions(+), 70 deletions(-) diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp index fd08b0239b..26d42ba59c 100644 --- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp +++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp @@ -992,9 +992,7 @@ static bool alreadyExecutedAtLeastOneLoopIteration(const ExplodedNode *N, ProgramPoint P = N->getLocation(); if (Optional BE = P.getAs()) { - if (BE->getSrc()->getLoopTarget() == FCS) - return true; - return false; + return BE->getSrc()->getLoopTarget() == FCS; } // Keep looking for a block edge. @@ -1038,11 +1036,8 @@ bool ObjCLoopChecker::isCollectionCountMethod(const ObjCMethodCall &M, CountSelectorII = &C.getASTContext().Idents.get("count"); // If the method returns collection count, record the value. - if (S.isUnarySelector() && - (S.getIdentifierInfoForSlot(0) == CountSelectorII)) - return true; - - return false; + return S.isUnarySelector() && + (S.getIdentifierInfoForSlot(0) == CountSelectorII); } void ObjCLoopChecker::checkPostObjCMessage(const ObjCMethodCall &M, diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index d32b282222..5d78d9b02e 100644 --- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -2013,10 +2013,7 @@ bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const { // properties are held. However, if the user chooses to turn off some of these // checks, we ignore the issues and leave the call evaluation to a generic // handler. - if (!C.isDifferent()) - return false; - - return true; + return C.isDifferent(); } void CStringChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const { diff --git a/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp b/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp index a5ed64d160..2337400750 100644 --- a/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp @@ -82,10 +82,7 @@ static bool evenFlexibleArraySize(ASTContext &Ctx, CharUnits RegionSize, if (Left.isNegative()) return false; - if (Left % FlexSize == 0) - return true; - - return false; + return Left % FlexSize == 0; } void CastSizeChecker::checkPreStmt(const CastExpr *CE,CheckerContext &C) const { diff --git a/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp index a71def23c0..ad478cbf78 100644 --- a/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp +++ b/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp @@ -41,13 +41,12 @@ namespace { /// Checks for the init, dealloc, and any other functions that might be allowed /// to perform direct instance variable assignment based on their name. static bool DefaultMethodFilter(const ObjCMethodDecl *M) { - if (M->getMethodFamily() == OMF_init || M->getMethodFamily() == OMF_dealloc || - M->getMethodFamily() == OMF_copy || - M->getMethodFamily() == OMF_mutableCopy || - M->getSelector().getNameForSlot(0).find("init") != StringRef::npos || - M->getSelector().getNameForSlot(0).find("Init") != StringRef::npos) - return true; - return false; + return M->getMethodFamily() == OMF_init || + M->getMethodFamily() == OMF_dealloc || + M->getMethodFamily() == OMF_copy || + M->getMethodFamily() == OMF_mutableCopy || + M->getSelector().getNameForSlot(0).find("init") != StringRef::npos || + M->getSelector().getNameForSlot(0).find("Init") != StringRef::npos; } class DirectIvarAssignment : diff --git a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 1d8085108c..8c8acc637f 100644 --- a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp @@ -658,10 +658,8 @@ bool GenericTaintChecker::checkUncontrolledFormatString(const CallExpr *CE, return false; // If either the format string content or the pointer itself are tainted, warn. - if (generateReportIfTainted(CE->getArg(ArgNum), - MsgUncontrolledFormatString, C)) - return true; - return false; + return generateReportIfTainted(CE->getArg(ArgNum), + MsgUncontrolledFormatString, C); } bool GenericTaintChecker::checkSystemCall(const CallExpr *CE, @@ -686,11 +684,7 @@ bool GenericTaintChecker::checkSystemCall(const CallExpr *CE, if (ArgNum == UINT_MAX || CE->getNumArgs() < (ArgNum + 1)) return false; - if (generateReportIfTainted(CE->getArg(ArgNum), - MsgSanitizeSystemArgs, C)) - return true; - - return false; + return generateReportIfTainted(CE->getArg(ArgNum), MsgSanitizeSystemArgs, C); } // TODO: Should this check be a part of the CString checker? @@ -728,11 +722,8 @@ bool GenericTaintChecker::checkTaintedBufferSize(const CallExpr *CE, ArgNum = 2; } - if (ArgNum != InvalidArgIndex && CE->getNumArgs() > ArgNum && - generateReportIfTainted(CE->getArg(ArgNum), MsgTaintedBufferSize, C)) - return true; - - return false; + return ArgNum != InvalidArgIndex && CE->getNumArgs() > ArgNum && + generateReportIfTainted(CE->getArg(ArgNum), MsgTaintedBufferSize, C); } void ento::registerGenericTaintChecker(CheckerManager &mgr) { diff --git a/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp b/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp index 6cfd07a9e9..0c3bff5b63 100644 --- a/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp @@ -305,9 +305,7 @@ static bool isIdenticalStmt(const ASTContext &Ctx, const Stmt *Stmt1, const Stmt *Stmt2, bool IgnoreSideEffects) { if (!Stmt1 || !Stmt2) { - if (!Stmt1 && !Stmt2) - return true; - return false; + return !Stmt1 && !Stmt2; } // If Stmt1 & Stmt2 are of different class then they are not diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index 294f3617d7..1e56d709e4 100644 --- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp @@ -201,12 +201,8 @@ unsigned MacOSKeychainAPIChecker::getTrackedFunctionIndex(StringRef Name, static bool isBadDeallocationArgument(const MemRegion *Arg) { if (!Arg) return false; - if (isa(Arg) || - isa(Arg) || - isa(Arg)) { - return true; - } - return false; + return isa(Arg) || isa(Arg) || + isa(Arg); } /// Given the address expression, retrieve the value it's pointing to. Assume @@ -240,11 +236,7 @@ bool MacOSKeychainAPIChecker::definitelyReturnedError(SymbolRef RetSym, DefinedOrUnknownSVal NoErr = Builder.evalEQ(State, NoErrVal, nonloc::SymbolVal(RetSym)); ProgramStateRef ErrState = State->assume(NoErr, noError); - if (ErrState == State) { - return true; - } - - return false; + return ErrState == State; } // Report deallocator mismatch. Remove the region from tracking - reporting a diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 54b1a6ee0f..713d9fe285 100644 --- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -1000,12 +1000,9 @@ static bool isKnownDeallocObjCMethodName(const ObjCMethodCall &Call) { // Ex: [NSData dataWithBytesNoCopy:bytes length:10]; // (...unless a 'freeWhenDone' parameter is false, but that's checked later.) StringRef FirstSlot = Call.getSelector().getNameForSlot(0); - if (FirstSlot == "dataWithBytesNoCopy" || - FirstSlot == "initWithBytesNoCopy" || - FirstSlot == "initWithCharactersNoCopy") - return true; - - return false; + return FirstSlot == "dataWithBytesNoCopy" || + FirstSlot == "initWithBytesNoCopy" || + FirstSlot == "initWithCharactersNoCopy"; } static Optional getFreeWhenDoneArg(const ObjCMethodCall &Call) { diff --git a/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp index 224251beb0..b10ec848ee 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCContainersASTChecker.cpp @@ -66,9 +66,8 @@ class WalkAST : public StmtVisitor { // The type must be an array/pointer type. // This could be a null constant, which is allowed. - if (E->isNullPointerConstant(ASTC, Expr::NPC_ValueDependentIsNull)) - return true; - return false; + return static_cast( + E->isNullPointerConstant(ASTC, Expr::NPC_ValueDependentIsNull)); } public: diff --git a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp index f344dd09c7..ffa3a27006 100644 --- a/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp @@ -404,10 +404,7 @@ static bool shouldRunOnFunctionOrMethod(const NamedDecl *ND) { if (II == NSObjectII) break; } - if (!ID) - return false; - - return true; + return ID != nullptr; } /// \brief Returns true if the location is 'self'. diff --git a/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp b/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp index 9f66dc9be4..a03abce962 100644 --- a/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp @@ -235,12 +235,9 @@ bool UnreachableCodeChecker::isInvalidPath(const CFGBlock *CB, return false; // Run each of the checks on the conditions - if (containsMacro(cond) || containsEnum(cond) - || containsStaticLocal(cond) || containsBuiltinOffsetOf(cond) - || containsStmt(cond)) - return true; - - return false; + return containsMacro(cond) || containsEnum(cond) || + containsStaticLocal(cond) || containsBuiltinOffsetOf(cond) || + containsStmt(cond); } // Returns true if the given CFGBlock is empty -- 2.50.1