[analyzer][NFC] Refactor llvm::isa<> usages in the StaticAnalyzer

It turns out llvm::isa<> is variadic, and we could have used this at a
lot of places.

The following patterns:
  x && isa<T1>(x) || isa<T2>(x) ...
Will be replaced by:
  isa_and_non_null<T1, T2, ...>(x)

Sometimes it caused further simplifications, when it would cause even
more code smell.

Aside from this, keep in mind that within `assert()` or any macro
functions, we need to wrap the isa<> expression within a parenthesis,
due to the parsing of the comma.

Reviewed By: martong

Differential Revision: https://reviews.llvm.org/D111982
This commit is contained in:
Balazs Benics 2021-10-20 17:43:31 +02:00
parent aab0f2264a
commit 16be17ad4b
22 changed files with 56 additions and 89 deletions

View File

@ -93,11 +93,10 @@ void AnalyzerStatsChecker::checkEndAnalysis(ExplodedGraph &G,
if (!Loc.isValid())
return;
if (isa<FunctionDecl>(D) || isa<ObjCMethodDecl>(D)) {
if (isa<FunctionDecl, ObjCMethodDecl>(D)) {
const NamedDecl *ND = cast<NamedDecl>(D);
output << *ND;
}
else if (isa<BlockDecl>(D)) {
} else if (isa<BlockDecl>(D)) {
output << "block(line:" << Loc.getLine() << ":col:" << Loc.getColumn();
}

View File

@ -160,7 +160,7 @@ static bool isEnclosingFunctionParam(const Expr *E) {
E = E->IgnoreParenCasts();
if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {
const ValueDecl *VD = DRE->getDecl();
if (isa<ImplicitParamDecl>(VD) || isa<ParmVarDecl>(VD))
if (isa<ImplicitParamDecl, ParmVarDecl>(VD))
return true;
}
return false;
@ -199,8 +199,7 @@ unsigned MacOSKeychainAPIChecker::getTrackedFunctionIndex(StringRef Name,
static bool isBadDeallocationArgument(const MemRegion *Arg) {
if (!Arg)
return false;
return isa<AllocaRegion>(Arg) || isa<BlockDataRegion>(Arg) ||
isa<TypedRegion>(Arg);
return isa<AllocaRegion, BlockDataRegion, TypedRegion>(Arg);
}
/// Given the address expression, retrieve the value it's pointing to. Assume

View File

@ -936,7 +936,7 @@ public:
/// Did not track -> allocated. Other state (released) -> allocated.
static inline bool isAllocated(const RefState *RSCurr, const RefState *RSPrev,
const Stmt *Stmt) {
return (Stmt && (isa<CallExpr>(Stmt) || isa<CXXNewExpr>(Stmt)) &&
return (isa_and_nonnull<CallExpr, CXXNewExpr>(Stmt) &&
(RSCurr &&
(RSCurr->isAllocated() || RSCurr->isAllocatedOfSizeZero())) &&
(!RSPrev ||
@ -949,8 +949,7 @@ public:
const Stmt *Stmt) {
bool IsReleased =
(RSCurr && RSCurr->isReleased()) && (!RSPrev || !RSPrev->isReleased());
assert(!IsReleased ||
(Stmt && (isa<CallExpr>(Stmt) || isa<CXXDeleteExpr>(Stmt))) ||
assert(!IsReleased || (isa_and_nonnull<CallExpr, CXXDeleteExpr>(Stmt)) ||
(!Stmt && RSCurr->getAllocationFamily() == AF_InnerBuffer));
return IsReleased;
}
@ -958,11 +957,10 @@ public:
/// Did not track -> relinquished. Other state (allocated) -> relinquished.
static inline bool isRelinquished(const RefState *RSCurr,
const RefState *RSPrev, const Stmt *Stmt) {
return (Stmt &&
(isa<CallExpr>(Stmt) || isa<ObjCMessageExpr>(Stmt) ||
isa<ObjCPropertyRefExpr>(Stmt)) &&
(RSCurr && RSCurr->isRelinquished()) &&
(!RSPrev || !RSPrev->isRelinquished()));
return (
isa_and_nonnull<CallExpr, ObjCMessageExpr, ObjCPropertyRefExpr>(Stmt) &&
(RSCurr && RSCurr->isRelinquished()) &&
(!RSPrev || !RSPrev->isRelinquished()));
}
/// If the expression is not a call, and the state change is
@ -972,7 +970,7 @@ public:
static inline bool hasReallocFailed(const RefState *RSCurr,
const RefState *RSPrev,
const Stmt *Stmt) {
return ((!Stmt || !isa<CallExpr>(Stmt)) &&
return ((!isa_and_nonnull<CallExpr>(Stmt)) &&
(RSCurr &&
(RSCurr->isAllocated() || RSCurr->isAllocatedOfSizeZero())) &&
(RSPrev &&
@ -1921,7 +1919,7 @@ ProgramStateRef MallocChecker::FreeMemAux(
// Parameters, locals, statics, globals, and memory returned by
// __builtin_alloca() shouldn't be freed.
if (!(isa<UnknownSpaceRegion>(MS) || isa<HeapSpaceRegion>(MS))) {
if (!isa<UnknownSpaceRegion, HeapSpaceRegion>(MS)) {
// FIXME: at the time this code was written, malloc() regions were
// represented by conjured symbols, which are all in UnknownSpaceRegion.
// This means that there isn't actually anything from HeapSpaceRegion
@ -2904,7 +2902,7 @@ void MallocChecker::checkEscapeOnReturn(const ReturnStmt *S,
// the callee could still free the memory.
// TODO: This logic should be a part of generic symbol escape callback.
if (const MemRegion *MR = RetVal.getAsRegion())
if (isa<FieldRegion>(MR) || isa<ElementRegion>(MR))
if (isa<FieldRegion, ElementRegion>(MR))
if (const SymbolicRegion *BMR =
dyn_cast<SymbolicRegion>(MR->getBaseRegion()))
Sym = BMR->getSymbol();
@ -3087,7 +3085,7 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(
// TODO: If we want to be more optimistic here, we'll need to make sure that
// regions escape to C++ containers. They seem to do that even now, but for
// mysterious reasons.
if (!(isa<SimpleFunctionCall>(Call) || isa<ObjCMethodCall>(Call)))
if (!isa<SimpleFunctionCall, ObjCMethodCall>(Call))
return true;
// Check Objective-C messages by selector name.

View File

@ -102,8 +102,7 @@ void MallocOverflowSecurityChecker::CheckMallocArgument(
e = rhs;
} else
return;
}
else if (isa<DeclRefExpr>(e) || isa<MemberExpr>(e))
} else if (isa<DeclRefExpr, MemberExpr>(e))
break;
else
return;

View File

@ -553,8 +553,8 @@ MoveChecker::classifyObject(const MemRegion *MR,
// For the purposes of this checker, we classify move-safe STL types
// as not-"STL" types, because that's how the checker treats them.
MR = unwrapRValueReferenceIndirection(MR);
bool IsLocal =
MR && isa<VarRegion>(MR) && isa<StackSpaceRegion>(MR->getMemorySpace());
bool IsLocal = isa_and_nonnull<VarRegion>(MR) &&
isa<StackSpaceRegion>(MR->getMemorySpace());
if (!RD || !RD->getDeclContext()->isStdNamespace())
return { IsLocal, SK_NonStd };

View File

@ -73,11 +73,8 @@ RefCountBug::RefCountBug(CheckerNameRef Checker, RefCountBugKind BT)
static bool isNumericLiteralExpression(const Expr *E) {
// FIXME: This set of cases was copied from SemaExprObjC.
return isa<IntegerLiteral>(E) ||
isa<CharacterLiteral>(E) ||
isa<FloatingLiteral>(E) ||
isa<ObjCBoolLiteralExpr>(E) ||
isa<CXXBoolLiteralExpr>(E);
return isa<IntegerLiteral, CharacterLiteral, FloatingLiteral,
ObjCBoolLiteralExpr, CXXBoolLiteralExpr>(E);
}
/// If type represents a pointer to CXXRecordDecl,

View File

@ -110,7 +110,7 @@ void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
// Don't treat functions in namespaces with the same name a Unix function
// as a call to the Unix function.
const DeclContext *NamespaceCtx = FD->getEnclosingNamespaceContext();
if (NamespaceCtx && isa<NamespaceDecl>(NamespaceCtx))
if (isa_and_nonnull<NamespaceDecl>(NamespaceCtx))
return;
StringRef FName = C.getCalleeName(FD);
@ -466,7 +466,7 @@ void UnixAPIPortabilityChecker::checkPreStmt(const CallExpr *CE,
// Don't treat functions in namespaces with the same name a Unix function
// as a call to the Unix function.
const DeclContext *NamespaceCtx = FD->getEnclosingNamespaceContext();
if (NamespaceCtx && isa<NamespaceDecl>(NamespaceCtx))
if (isa_and_nonnull<NamespaceDecl>(NamespaceCtx))
return;
StringRef FName = C.getCalleeName(FD);

View File

@ -537,10 +537,10 @@ static void removeEdgesToDefaultInitializers(PathPieces &Pieces) {
if (auto *CF = dyn_cast<PathDiagnosticControlFlowPiece>(I->get())) {
const Stmt *Start = CF->getStartLocation().asStmt();
const Stmt *End = CF->getEndLocation().asStmt();
if (Start && isa<CXXDefaultInitExpr>(Start)) {
if (isa_and_nonnull<CXXDefaultInitExpr>(Start)) {
I = Pieces.erase(I);
continue;
} else if (End && isa<CXXDefaultInitExpr>(End)) {
} else if (isa_and_nonnull<CXXDefaultInitExpr>(End)) {
PathPieces::iterator Next = std::next(I);
if (Next != E) {
if (auto *NextCF =
@ -1314,8 +1314,7 @@ void PathDiagnosticBuilder::generatePathDiagnosticsForNode(
C.getActivePath().push_front(std::move(PE));
}
}
} else if (isa<BreakStmt>(Term) || isa<ContinueStmt>(Term) ||
isa<GotoStmt>(Term)) {
} else if (isa<BreakStmt, ContinueStmt, GotoStmt>(Term)) {
PathDiagnosticLocation L(Term, SM, C.getCurrLocationContext());
addEdgeToPath(C.getActivePath(), PrevLoc, L);
}
@ -1354,9 +1353,7 @@ static const Stmt *getStmtParent(const Stmt *S, const ParentMap &PM) {
if (!S)
break;
if (isa<FullExpr>(S) ||
isa<CXXBindTemporaryExpr>(S) ||
isa<SubstNonTypeTemplateParmExpr>(S))
if (isa<FullExpr, CXXBindTemporaryExpr, SubstNonTypeTemplateParmExpr>(S))
continue;
break;
@ -1552,9 +1549,8 @@ static void simplifySimpleBranches(PathPieces &pieces) {
// We only perform this transformation for specific branch kinds.
// We don't want to do this for do..while, for example.
if (!(isa<ForStmt>(s1Start) || isa<WhileStmt>(s1Start) ||
isa<IfStmt>(s1Start) || isa<ObjCForCollectionStmt>(s1Start) ||
isa<CXXForRangeStmt>(s1Start)))
if (!isa<ForStmt, WhileStmt, IfStmt, ObjCForCollectionStmt,
CXXForRangeStmt>(s1Start))
continue;
// Is s1End the branch condition?
@ -3193,7 +3189,7 @@ findExecutedLines(const SourceManager &SM, const ExplodedNode *N) {
P = N->getParentMap().getParent(RS);
}
if (P && (isa<SwitchCase>(P) || isa<LabelStmt>(P)))
if (isa_and_nonnull<SwitchCase, LabelStmt>(P))
populateExecutedLinesWithStmt(P, SM, *ExecutedLines);
}

View File

@ -815,7 +815,7 @@ bool NoStoreFuncVisitor::prettyPrintRegionName(const RegionVector &FieldChain,
// Just keep going up to the base region.
// Element regions may appear due to casts.
if (isa<CXXBaseObjectRegion>(R) || isa<CXXTempObjectRegion>(R))
if (isa<CXXBaseObjectRegion, CXXTempObjectRegion>(R))
continue;
if (Sep.empty())
@ -2735,9 +2735,8 @@ bool ConditionBRVisitor::patternMatch(const Expr *Ex,
const Expr *OriginalExpr = Ex;
Ex = Ex->IgnoreParenCasts();
if (isa<GNUNullExpr>(Ex) || isa<ObjCBoolLiteralExpr>(Ex) ||
isa<CXXBoolLiteralExpr>(Ex) || isa<IntegerLiteral>(Ex) ||
isa<FloatingLiteral>(Ex)) {
if (isa<GNUNullExpr, ObjCBoolLiteralExpr, CXXBoolLiteralExpr, IntegerLiteral,
FloatingLiteral>(Ex)) {
// Use heuristics to determine if the expression is a macro
// expanding to a literal and if so, use the macro's name.
SourceLocation BeginLoc = OriginalExpr->getBeginLoc();

View File

@ -425,9 +425,7 @@ void CallEvent::dump(raw_ostream &Out) const {
}
bool CallEvent::isCallStmt(const Stmt *S) {
return isa<CallExpr>(S) || isa<ObjCMessageExpr>(S)
|| isa<CXXConstructExpr>(S)
|| isa<CXXNewExpr>(S);
return isa<CallExpr, ObjCMessageExpr, CXXConstructExpr, CXXNewExpr>(S);
}
QualType CallEvent::getDeclaredResultType(const Decl *D) {

View File

@ -38,7 +38,7 @@ StringRef CheckerContext::getCalleeName(const FunctionDecl *FunDecl) const {
}
StringRef CheckerContext::getDeclDescription(const Decl *D) {
if (isa<ObjCMethodDecl>(D) || isa<CXXMethodDecl>(D))
if (isa<ObjCMethodDecl, CXXMethodDecl>(D))
return "method";
if (isa<BlockDecl>(D))
return "anonymous block";

View File

@ -88,7 +88,7 @@ SVal Environment::getSVal(const EnvironmentEntry &Entry,
const Stmt *S = Entry.getStmt();
assert(!isa<ObjCForCollectionStmt>(S) &&
"Use ExprEngine::hasMoreIteration()!");
assert((isa<Expr>(S) || isa<ReturnStmt>(S)) &&
assert((isa<Expr, ReturnStmt>(S)) &&
"Environment can only argue about Exprs, since only they express "
"a value! Any non-expression statement stored in Environment is a "
"result of a hack!");

View File

@ -50,8 +50,7 @@ ExplodedGraph::~ExplodedGraph() = default;
bool ExplodedGraph::isInterestingLValueExpr(const Expr *Ex) {
if (!Ex->isLValue())
return false;
return isa<DeclRefExpr>(Ex) || isa<MemberExpr>(Ex) ||
isa<ObjCIvarRefExpr>(Ex) || isa<ArraySubscriptExpr>(Ex);
return isa<DeclRefExpr, MemberExpr, ObjCIvarRefExpr, ArraySubscriptExpr>(Ex);
}
bool ExplodedGraph::shouldCollect(const ExplodedNode *node) {

View File

@ -1989,8 +1989,7 @@ void ExprEngine::processCFGBlockEntrance(const BlockEdge &L,
if (BlockCount == AMgr.options.maxBlockVisitOnPath - 1 &&
AMgr.options.ShouldWidenLoops) {
const Stmt *Term = nodeBuilder.getContext().getBlock()->getTerminatorStmt();
if (!(Term &&
(isa<ForStmt>(Term) || isa<WhileStmt>(Term) || isa<DoStmt>(Term))))
if (!isa_and_nonnull<ForStmt, WhileStmt, DoStmt>(Term))
return;
// Widen.
const LocationContext *LCtx = Pred->getLocationContext();
@ -2266,7 +2265,7 @@ void ExprEngine::processBranch(const Stmt *Condition,
continue;
}
if (StTrue && StFalse)
assert(!isa<ObjCForCollectionStmt>(Condition));;
assert(!isa<ObjCForCollectionStmt>(Condition));
// Process the true branch.
if (builder.isFeasible(true)) {
@ -2594,7 +2593,7 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
ProgramPoint::PostLValueKind);
return;
}
if (isa<FieldDecl>(D) || isa<IndirectFieldDecl>(D)) {
if (isa<FieldDecl, IndirectFieldDecl>(D)) {
// Delegate all work related to pointer to members to the surrounding
// operator&.
return;
@ -2671,7 +2670,7 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred,
// Handle static member variables and enum constants accessed via
// member syntax.
if (isa<VarDecl>(Member) || isa<EnumConstantDecl>(Member)) {
if (isa<VarDecl, EnumConstantDecl>(Member)) {
for (const auto I : CheckedSet)
VisitCommonDeclRefExpr(M, Member, I, EvalSet);
} else {

View File

@ -550,7 +550,7 @@ void ExprEngine::VisitCompoundLiteralExpr(const CompoundLiteralExpr *CL,
const Expr *Init = CL->getInitializer();
SVal V = State->getSVal(CL->getInitializer(), LCtx);
if (isa<CXXConstructExpr>(Init) || isa<CXXStdInitializerListExpr>(Init)) {
if (isa<CXXConstructExpr, CXXStdInitializerListExpr>(Init)) {
// No work needed. Just pass the value up to this expression.
} else {
assert(isa<InitListExpr>(Init));
@ -984,8 +984,7 @@ void ExprEngine::VisitUnaryOperator(const UnaryOperator* U, ExplodedNode *Pred,
if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Ex)) {
const ValueDecl *VD = DRE->getDecl();
if (isa<CXXMethodDecl>(VD) || isa<FieldDecl>(VD) ||
isa<IndirectFieldDecl>(VD)) {
if (isa<CXXMethodDecl, FieldDecl, IndirectFieldDecl>(VD)) {
ProgramStateRef State = (*I)->getState();
const LocationContext *LCtx = (*I)->getLocationContext();
SVal SV = svalBuilder.getMemberPointer(cast<NamedDecl>(VD));

View File

@ -69,7 +69,7 @@ namespace clang {
namespace ento {
static bool isLoopStmt(const Stmt *S) {
return S && (isa<ForStmt>(S) || isa<WhileStmt>(S) || isa<DoStmt>(S));
return isa_and_nonnull<ForStmt, WhileStmt, DoStmt>(S);
}
ProgramStateRef processLoopEnd(const Stmt *LoopStmt, ProgramStateRef State) {

View File

@ -45,8 +45,7 @@ ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
const LocationContext *LCtx,
unsigned BlockCount, const Stmt *LoopStmt) {
assert(isa<ForStmt>(LoopStmt) || isa<WhileStmt>(LoopStmt) ||
isa<DoStmt>(LoopStmt));
assert((isa<ForStmt, WhileStmt, DoStmt>(LoopStmt)));
// Invalidate values in the current state.
// TODO Make this more conservative by only invalidating values that might

View File

@ -1012,14 +1012,15 @@ const VarRegion *MemRegionManager::getVarRegion(const VarDecl *D,
sReg = getUnknownRegion();
} else {
if (D->hasLocalStorage()) {
sReg = isa<ParmVarDecl>(D) || isa<ImplicitParamDecl>(D)
? static_cast<const MemRegion*>(getStackArgumentsRegion(STC))
: static_cast<const MemRegion*>(getStackLocalsRegion(STC));
sReg =
isa<ParmVarDecl, ImplicitParamDecl>(D)
? static_cast<const MemRegion *>(getStackArgumentsRegion(STC))
: static_cast<const MemRegion *>(getStackLocalsRegion(STC));
}
else {
assert(D->isStaticLocal());
const Decl *STCD = STC->getDecl();
if (isa<FunctionDecl>(STCD) || isa<ObjCMethodDecl>(STCD))
if (isa<FunctionDecl, ObjCMethodDecl>(STCD))
sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
getFunctionCodeRegion(cast<NamedDecl>(STCD)));
else if (const auto *BD = dyn_cast<BlockDecl>(STCD)) {
@ -1283,9 +1284,7 @@ bool MemRegion::hasStackParametersStorage() const {
}
bool MemRegion::hasGlobalsOrParametersStorage() const {
const MemSpaceRegion *MS = getMemorySpace();
return isa<StackArgumentsSpaceRegion>(MS) ||
isa<GlobalsSpaceRegion>(MS);
return isa<StackArgumentsSpaceRegion, GlobalsSpaceRegion>(getMemorySpace());
}
// getBaseRegion strips away all elements and fields, and get the base region

View File

@ -62,8 +62,8 @@ private:
: P(r, k), Data(offset) {
assert(r && "Must have known regions.");
assert(getOffset() == offset && "Failed to store offset");
assert((r == r->getBaseRegion() || isa<ObjCIvarRegion>(r) ||
isa <CXXDerivedObjectRegion>(r)) &&
assert((r == r->getBaseRegion() ||
isa<ObjCIvarRegion, CXXDerivedObjectRegion>(r)) &&
"Not a base");
}
public:
@ -1135,7 +1135,7 @@ void InvalidateRegionsWorker::VisitCluster(const MemRegion *baseR,
if (Regions)
Regions->push_back(baseR);
if (isa<AllocaRegion>(baseR) || isa<SymbolicRegion>(baseR)) {
if (isa<AllocaRegion, SymbolicRegion>(baseR)) {
// Invalidate the region by setting its default value to
// conjured symbol. The type of the symbol is irrelevant.
DefinedOrUnknownSVal V =
@ -1224,7 +1224,7 @@ void InvalidateRegionsWorker::VisitCluster(const MemRegion *baseR,
// detection.
SVal V = I.getData();
const MemRegion *R = V.getAsRegion();
if (R && isa<SymbolicRegion>(R))
if (isa_and_nonnull<SymbolicRegion>(R))
VisitBinding(V);
}
}

View File

@ -254,8 +254,7 @@ SValBuilder::getDerivedRegionValueSymbolVal(SymbolRef parentSymbol,
}
DefinedSVal SValBuilder::getMemberPointer(const NamedDecl *ND) {
assert(!ND || isa<CXXMethodDecl>(ND) || isa<FieldDecl>(ND) ||
isa<IndirectFieldDecl>(ND));
assert(!ND || (isa<CXXMethodDecl, FieldDecl, IndirectFieldDecl>(ND)));
if (const auto *MD = dyn_cast_or_null<CXXMethodDecl>(ND)) {
// Sema treats pointers to static member functions as have function pointer

View File

@ -84,7 +84,7 @@ Optional<const MemRegion *> StoreManager::castRegion(const MemRegion *R,
// involved. Blocks can be casted to/from 'id', as they can be treated
// as Objective-C objects. This could possibly be handled by enhancing
// our reasoning of downcasts of symbolic objects.
if (isa<CodeTextRegion>(R) || isa<SymbolicRegion>(R))
if (isa<CodeTextRegion, SymbolicRegion>(R))
return R;
// We don't know what to make of it. Return a NULL region, which

View File

@ -425,19 +425,7 @@ bool SymbolReaper::isLiveRegion(const MemRegion *MR) {
// tell if anything still refers to this region. Unlike SymbolicRegions,
// AllocaRegions don't have associated symbols, though, so we don't actually
// have a way to track their liveness.
if (isa<AllocaRegion>(MR))
return true;
if (isa<CXXThisRegion>(MR))
return true;
if (isa<MemSpaceRegion>(MR))
return true;
if (isa<CodeTextRegion>(MR))
return true;
return false;
return isa<AllocaRegion, CXXThisRegion, MemSpaceRegion, CodeTextRegion>(MR);
}
bool SymbolReaper::isLive(SymbolRef sym) {