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

[steakhal]

Summary

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.

Note: This is an NFC change!
Please, review accordingly. I hope I did not mess up anything.

[whisperity]

isa<> became a variadic template a year or so ago, only!

I think in general you did everything right!

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

939

Hrm, I didn't know about isa_and_nonnull either...

952–953

I think keeping the (not null) Stmt check here would keep the code more readable. I had to stop and think about why you would be checking for Stmt being null while you go and dereference something else.

clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

14

What required this header / change?

clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp

18

Ditto.

[steakhal]

Thanks for the review @whisperity !
I really wanted to doublecheck this stuff, since I did this by hand.
Although, one can probably easily extend the existing llvm convention tidy checker to recognize this pattern.
That would make this refactoring really suitable for the whole llvm project family - even for downstream users.
However, I'm not the one implementing it :D

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

952–953

I like the new form.
And, just imagine if we would extend the llvm-prefer-isa-or-dyn-cast-in-conditionals checker with this pattern.
By simply applying the fixits we would reach this as its fixedpoint.
First, by merging the two distinct isa<> into a single one, then removing the redundant parenthesis.
Second, merging the Stmt && isa<> into an isa_and_non_null<>.

clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

14

Uh, I'm not sure. clang must have done something :D
Let me investigate!

clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp

18

Yea, yea. I'm on it.

[whisperity]

clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

14

I think in general your editor just decided including this header would be better than including llvm/Support/Casting.h.

[whisperity]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

952–953

It seems that's already implemented and always has been since the introduction of the check (rG28413dd87aa21):

  } else if (const auto *MatchedDecl =
                 Result.Nodes.getNodeAs<BinaryOperator>("and")) {

/* ... */

    std::string Replacement("isa_and_nonnull");
    Replacement += RHSString.substr(Func->getName().size());

    diag(MatchedDecl->getBeginLoc(),
         "isa_and_nonnull<> is preferred over an explicit test for null "
         "followed by calling isa<>")
        << FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(),
                                                    MatchedDecl->getEndLoc()),
                                        Replacement);

[steakhal]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

952–953

I meant the disjunction of isa<>s - which is probably not implemented yet.

[martong]

Looks good, but please take care of the header inclusion problem.

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

939

Ahh, this is very awesome.

clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp

15

This header inclusion now wreaks havoc the alphabetical ordering.

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

23

One more.

clang/lib/StaticAnalyzer/Core/SymbolManager.cpp

428

Nice!

[martong]

And @whisperity , thanks for the review!

clang/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp

void AnalyzerStatsChecker::checkEndAnalysis(ExplodedGraph &G,
 
   // Generate the warning string
   SmallString<128> buf;
   llvm::raw_svector_ostream output(buf);
   PresumedLoc Loc = SM.getPresumedLoc(D->getLocation());
   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();
   }
 
   NumBlocksUnreachable += unreachable;
   NumBlocks += total;
   std::string NameOfRootFunction = std::string(output.str());
 
   output << " -> Total CFGBlocks: " << total << " | Unreachable CFGBlocks: "

clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp

 REGISTER_MAP_WITH_PROGRAMSTATE(AllocatedData,
                                SymbolRef,
                                MacOSKeychainAPIChecker::AllocationState)
 
 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;
 }
 
 const MacOSKeychainAPIChecker::ADFunctionInfo
   MacOSKeychainAPIChecker::FunctionsToTrack[FunctionsToTrackSize] = {
     {"SecKeychainItemCopyContent", 4, 3, ValidAPI},                       // 0
unsigned MacOSKeychainAPIChecker::getTrackedFunctionIndex(StringRef Name,
   }
   // The function is not tracked.
   return InvalidIdx;
 }
 
 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
 /// that value is itself an address, and return the corresponding symbol.
 static SymbolRef getAsPointeeSymbol(const Expr *Expr,
                                     CheckerContext &C) {
   ProgramStateRef State = C.getState();
   SVal ArgV = C.getSVal(Expr);

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

public:
   void Profile(llvm::FoldingSetNodeID &ID) const override {
     ID.AddPointer(getTag());
     ID.AddPointer(Sym);
   }
 
   /// 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 ||
              !(RSPrev->isAllocated() || RSPrev->isAllocatedOfSizeZero())));
   }
 
   /// Did not track -> released. Other state (allocated) -> released.
   /// The statement associated with the release might be missing.
   static inline bool isReleased(const RefState *RSCurr, const RefState *RSPrev,
                                 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;
   }
 
   /// 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) &&
   }
 
   /// If the expression is not a call, and the state change is
   /// released -> allocated, it must be the realloc return value
   /// check. If we have to handle more cases here, it might be cleaner just
   /// to track this extra bit in the state itself.
   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 &&
              !(RSPrev->isAllocated() || RSPrev->isAllocatedOfSizeZero())));
   }
 
   PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
                                    BugReporterContext &BRC,
HandleNonHeapDealloc(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
                          Family);
     return nullptr;
   }
 
   const MemSpaceRegion *MS = R->getMemorySpace();
 
   // 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
     // that should be freed, even though we allow it here.
     // Of course, free() can work on memory allocated outside the current
     // function, so UnknownSpaceRegion is always a possibility.
     // False negatives are better than false positives.
 
void MallocChecker::checkEscapeOnReturn(const ReturnStmt *S,
   ProgramStateRef State = C.getState();
   SVal RetVal = C.getSVal(E);
   SymbolRef Sym = RetVal.getAsSymbol();
   if (!Sym)
     // If we are returning a field of the allocated struct or an array element,
     // 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();
 
   // Check if we are returning freed memory.
   if (Sym)
     checkUseAfterFree(Sym, C, E);
 }
bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(
                                               SymbolRef &EscapingSymbol) const {
   assert(Call);
   EscapingSymbol = nullptr;
 
   // For now, assume that any C++ or block call can free memory.
   // 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.
   if (const ObjCMethodCall *Msg = dyn_cast<ObjCMethodCall>(Call)) {
     // If it's not a framework call, or if it takes a callback, assume it
     // can free memory.
     if (!Call->isInSystemHeader() || Call->argumentsMayEscape())
       return true;

clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp

if (const BinaryOperator *binop = dyn_cast<BinaryOperator>(e)) {
       } else if ((opc == BO_Add || opc == BO_Mul) &&
                  lhs->isEvaluatable(Context)) {
         maxVal = lhs->EvaluateKnownConstInt(Context);
         if (EvaluatesToZero(maxVal, opc))
           return;
         e = rhs;
       } else
         return;
-    }
-    else if (isa<DeclRefExpr>(e) || isa<MemberExpr>(e))
+    } else if (isa<DeclRefExpr, MemberExpr>(e))
       break;
     else
       return;
   }
 
   if (mulop == nullptr)
     return;
 

clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp

 
 MoveChecker::ObjectKind
 MoveChecker::classifyObject(const MemRegion *MR,
                             const CXXRecordDecl *RD) const {
   // Local variables and local rvalue references are classified as "Local".
   // 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 };
 
   if (belongsTo(RD, StdSmartPtrClasses))
     return { IsLocal, SK_SmartPtr };
 
   if (belongsTo(RD, StdSafeClasses))

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

 RefCountBug::RefCountBug(CheckerNameRef Checker, RefCountBugKind BT)
     : BugType(Checker, bugTypeToName(BT), categories::MemoryRefCount,
               /*SuppressOnSink=*/BT == LeakWithinFunction ||
                   BT == LeakAtReturn),
       BT(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,
 /// and is not a typedef, return the decl name.
 /// Otherwise, return the serialization of type.
 static std::string getPrettyTypeName(QualType QT) {
   QualType PT = QT->getPointeeType();
   if (!PT.isNull() && !QT->getAs<TypedefType>())

clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

 //= UnixAPIChecker.h - Checks preconditions for various Unix APIs --*- C++ -*-//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This defines UnixAPIChecker, which is an assortment of checks on calls
 // to various, widely used UNIX/Posix functions.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
                                         CheckerContext &C) const {
   const FunctionDecl *FD = C.getCalleeDecl(CE);
   if (!FD || FD->getKind() != Decl::Function)
     return;
 
   // 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);
   if (FName.empty())
     return;
 
   if (FName == "open")
     CheckOpen(C, CE);
void UnixAPIPortabilityChecker::checkPreStmt(const CallExpr *CE,
                                              CheckerContext &C) const {
   const FunctionDecl *FD = C.getCalleeDecl(CE);
   if (!FD || FD->getKind() != Decl::Function)
     return;
 
   // 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);
   if (FName.empty())
     return;
 
   if (FName == "calloc")
     CheckCallocZero(C, CE);

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

if (auto *C = dyn_cast<PathDiagnosticCallPiece>(I->get()))
       removeEdgesToDefaultInitializers(C->path);
 
     if (auto *M = dyn_cast<PathDiagnosticMacroPiece>(I->get()))
       removeEdgesToDefaultInitializers(M->subPieces);
 
     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 =
                   dyn_cast<PathDiagnosticControlFlowPiece>(Next->get())) {
             NextCF->setStartLocation(CF->getStartLocation());
           }
         }
         I = Pieces.erase(I);
if (const Stmt *Term = BSrc->getTerminatorStmt()) {
           PE->setPrunable(true);
           addEdgeToPath(C.getActivePath(), PrevLoc, PE->getLocation());
 
           // We might've added a very similar control node already
           if (!C.shouldAddControlNotes()) {
             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);
       }
     }
   }
 }
 
 static std::unique_ptr<PathDiagnostic>
if (!S)
     return nullptr;
 
   while (true) {
     S = PM.getParentIgnoreParens(S);
 
     if (!S)
       break;
 
-    if (isa<FullExpr>(S) ||
-        isa<CXXBindTemporaryExpr>(S) ||
-        isa<SubstNonTypeTemplateParmExpr>(S))
+    if (isa<FullExpr, CXXBindTemporaryExpr, SubstNonTypeTemplateParmExpr>(S))
       continue;
 
     break;
   }
 
   return S;
 }
 
for (PathPieces::iterator I = pieces.begin(), E = pieces.end(); I != E; ++I) {
     const Stmt *s2Start = PieceNextI->getStartLocation().getStmtOrNull();
     const Stmt *s2End   = PieceNextI->getEndLocation().getStmtOrNull();
 
     if (!s2Start || !s2End || s1End != s2Start)
       continue;
 
     // 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?
     if (!isConditionForTerminator(s1Start, s1End))
       continue;
 
     // Perform the hoisting by eliminating (2) and changing the start
     // location of (3).
if (N->getFirstPred() == nullptr) {
       // The path exploration can die before the node with the associated
       // return statement is generated, but we do want to show the whole
       // return.
       if (const auto *RS = dyn_cast_or_null<ReturnStmt>(P)) {
         populateExecutedLinesWithStmt(RS, SM, *ExecutedLines);
         P = N->getParentMap().getParent(RS);
       }
 
-      if (P && (isa<SwitchCase>(P) || isa<LabelStmt>(P)))
+      if (isa_and_nonnull<SwitchCase, LabelStmt>(P))
         populateExecutedLinesWithStmt(P, SM, *ExecutedLines);
     }
 
     N = N->getFirstPred();
   }
   return ExecutedLines;
 }
 

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

bool NoStoreFuncVisitor::prettyPrintRegionName(const RegionVector &FieldChain,
   std::reverse(RegionSequence.begin(), RegionSequence.end());
   RegionSequence.append(FieldChain.begin(), FieldChain.end());
 
   StringRef Sep;
   for (const MemRegion *R : RegionSequence) {
 
     // 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())
       Sep = prettyPrintFirstElement(FirstElement,
                                     /*MoreItemsExpected=*/true,
                                     IndirectionLevel, os);
 
     os << Sep;
bool ConditionBRVisitor::patternMatch(const Expr *Ex,
                                       BugReporterContext &BRC,
                                       PathSensitiveBugReport &report,
                                       const ExplodedNode *N,
                                       Optional<bool> &prunable,
                                       bool IsSameFieldName) {
   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();
     SourceLocation EndLoc = OriginalExpr->getEndLoc();
     if (BeginLoc.isMacroID() && EndLoc.isMacroID()) {
       const SourceManager &SM = BRC.getSourceManager();
       const LangOptions &LO = BRC.getASTContext().getLangOpts();
       if (Lexer::isAtStartOfMacroExpansion(BeginLoc, SM, LO) &&

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

if (const Decl *D = getDecl()) {
     D->print(Out, Ctx.getPrintingPolicy());
     return;
   }
 
   Out << "Unknown call (type " << getKindAsString() << ")";
 }
 
 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) {
   assert(D);
   if (const auto *FD = dyn_cast<FunctionDecl>(D))
     return FD->getReturnType();
   if (const auto *MD = dyn_cast<ObjCMethodDecl>(D))
     return MD->getReturnType();

clang/lib/StaticAnalyzer/Core/CheckerContext.cpp

if (!FunDecl)
     return StringRef();
   IdentifierInfo *funI = FunDecl->getIdentifier();
   if (!funI)
     return StringRef();
   return funI->getName();
 }
 
 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";
   return "function";
 }
 
 bool CheckerContext::isCLibraryFunction(const FunctionDecl *FD,
                                         StringRef Name) {

clang/lib/StaticAnalyzer/Core/Environment.cpp

SVal Environment::lookupExpr(const EnvironmentEntry &E) const {
   return UnknownVal();
 }
 
 SVal Environment::getSVal(const EnvironmentEntry &Entry,
                           SValBuilder& svalBuilder) const {
   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!");
   const LocationContext *LCtx = Entry.getLocationContext();
 
   switch (S->getStmtClass()) {
   case Stmt::CXXBindTemporaryExprClass:
   case Stmt::ExprWithCleanupsClass:

clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp

 
 //===----------------------------------------------------------------------===//
 // Node reclamation.
 //===----------------------------------------------------------------------===//
 
 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) {
   // First, we only consider nodes for reclamation of the following
   // conditions apply:
   //
   // (1) 1 predecessor (that has one successor)
   // (2) 1 successor (that has one predecessor)

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

void ExprEngine::processCFGBlockEntrance(const BlockEdge &L,
   }
 
   // If this block is terminated by a loop and it has already been visited the
   // maximum number of times, widen the loop.
   unsigned int BlockCount = nodeBuilder.getContext().blockCount();
   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();
     ProgramStateRef WidenedState =
         getWidenedLoopState(Pred->getState(), LCtx, BlockCount, Term);
     nodeBuilder.generateNode(WidenedState, Pred);
     return;
   }
if (const auto KnownCondValueAssumption = assumeCondition(Condition, PredN))
       std::tie(StTrue, StFalse) = *KnownCondValueAssumption;
     else {
       assert(!isa<ObjCForCollectionStmt>(Condition));
       builder.generateNode(PrevState, true, PredN);
       builder.generateNode(PrevState, false, PredN);
       continue;
     }
     if (StTrue && StFalse)
-      assert(!isa<ObjCForCollectionStmt>(Condition));;
+      assert(!isa<ObjCForCollectionStmt>(Condition));
 
     // Process the true branch.
     if (builder.isFeasible(true)) {
       if (StTrue)
         builder.generateNode(StTrue, true, PredN);
       else
         builder.markInfeasible(true);
     }
if (const auto *ED = dyn_cast<EnumConstantDecl>(D)) {
     return;
   }
   if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
     SVal V = svalBuilder.getFunctionPointer(FD);
     Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
                       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;
   }
   if (isa<BindingDecl>(D)) {
     // FIXME: proper support for bound declarations.
     // For now, let's just prevent crashing.
     return;
void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred,
   ExplodedNodeSet CheckedSet;
   getCheckerManager().runCheckersForPreStmt(CheckedSet, Pred, M, *this);
 
   ExplodedNodeSet EvalSet;
   ValueDecl *Member = M->getMemberDecl();
 
   // 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 {
     StmtNodeBuilder Bldr(CheckedSet, EvalSet, *currBldrCtx);
     ExplodedNodeSet Tmp;
 
     for (const auto I : CheckedSet) {
       ProgramStateRef state = I->getState();

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

void ExprEngine::VisitCompoundLiteralExpr(const CompoundLiteralExpr *CL,
   StmtNodeBuilder B(Pred, Dst, *currBldrCtx);
 
   ProgramStateRef State = Pred->getState();
   const LocationContext *LCtx = Pred->getLocationContext();
 
   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));
     Loc CLLoc = State->getLValue(CL, LCtx);
     State = State->bindLoc(CLLoc, V, LCtx);
 
     if (CL->isGLValue())
       V = CLLoc;
for (ExplodedNodeSet::iterator I = CheckedSet.begin(), E = CheckedSet.end();
     }
 
     case UO_AddrOf: {
       // Process pointer-to-member address operation.
       const Expr *Ex = U->getSubExpr()->IgnoreParens();
       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));
           Bldr.generateNode(U, *I, State->BindExpr(U, LCtx, SV));
           break;
         }
       }
       // Explicitly proceed with default handler for this case cascade.

clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp

 //===--- LoopUnrolling.cpp - Unroll loops -----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// This file contains functions which are used to decide if a loop worth to be
 /// unrolled. Moreover, these functions manages the stack of loop which is
 /// tracked by the ProgramState.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h"
 
 using namespace clang;
 using namespace ento;
 using namespace clang::ast_matchers;
 
 static const int MAXIMUM_STEP_UNROLLED = 128;
 

 // lexical in nature. Instead, the stack of loops should be tracked in the
 // LocationContext.
 REGISTER_LIST_WITH_PROGRAMSTATE(LoopStack, LoopState)
 
 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) {
   auto LS = State->get<LoopStack>();
   if (!LS.isEmpty() && LS.getHead().getLoopStmt() == LoopStmt)
     State = State->set<LoopStack>(LS.getTail());
   return State;
 }

clang/lib/StaticAnalyzer/Core/LoopWidening.cpp

 
 namespace clang {
 namespace ento {
 
 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
   //      be modified by the body of the loop.
   // TODO Nested loops are currently widened as a result of the invalidation
   //      being so inprecise. When the invalidation is improved, the handling
   //      of nested loops will also need to be improved.
   ASTContext &ASTCtx = LCtx->getAnalysisDeclContext()->getASTContext();

clang/lib/StaticAnalyzer/Core/MemRegion.cpp

// Finally handle static locals.
     const auto *STC = V.get<const StackFrameContext *>();
 
     if (!STC) {
       // FIXME: Assign a more sensible memory space to static locals
       // we see from within blocks that we analyze as top-level declarations.
       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)) {
           // FIXME: The fallback type here is totally bogus -- though it should
           // never be queried, it will prevent uniquing with the real
           // BlockCodeRegion. Ideally we'd fix the AST so that we always had a
           // signature.
           QualType T;
bool MemRegion::hasStackNonParametersStorage() const {
   return isa<StackLocalsSpaceRegion>(getMemorySpace());
 }
 
 bool MemRegion::hasStackParametersStorage() const {
   return isa<StackArgumentsSpaceRegion>(getMemorySpace());
 }
 
 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
 // of them.
 const MemRegion *MemRegion::getBaseRegion() const {
   const MemRegion *R = this;
   while (true) {
     switch (R->getKind()) {

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/Attr.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Basic/JsonSupport.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "llvm/ADT/ImmutableMap.h"
 #include "llvm/ADT/Optional.h"
explicit BindingKey(const SubRegion *r, const SubRegion *Base, Kind k)
     assert(getConcreteOffsetRegion() == Base && "Failed to store base region");
   }
 
   /// Create a key for a binding at \p offset from base region \p r.
   explicit BindingKey(const MemRegion *r, uint64_t offset, Kind k)
     : 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:
 
   bool isDirect() const { return P.getInt() & Direct; }
   bool hasSymbolicOffset() const { return P.getInt() & Symbolic; }
 
   const MemRegion *getRegion() const { return P.getPointer(); }
void InvalidateRegionsWorker::VisitCluster(const MemRegion *baseR,
   // Nothing else should be done in the case when we preserve regions context.
   if (PreserveRegionsContents)
     return;
 
   // Otherwise, we have a normal data region. Record that we touched the region.
   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 =
       svalBuilder.conjureSymbolVal(baseR, Ex, LCtx, Ctx.IntTy, Count);
     B = B.addBinding(baseR, BindingKey::Default, V);
     return;
   }
 
if (doNotInvalidateSuperRegion) {
              (UpperOverflow &&
               (*ROffset >= LowerOffset || *ROffset < UpperOffset)) ||
              (LowerOffset == UpperOffset && *ROffset == LowerOffset))) {
           B = B.removeBinding(I.getKey());
           // Bound symbolic regions need to be invalidated for dead symbol
           // detection.
           SVal V = I.getData();
           const MemRegion *R = V.getAsRegion();
-          if (R && isa<SymbolicRegion>(R))
+          if (isa_and_nonnull<SymbolicRegion>(R))
             VisitBinding(V);
         }
       }
     }
   conjure_default:
       // Set the default value of the array to conjured symbol.
     DefinedOrUnknownSVal V =
     svalBuilder.conjureSymbolVal(baseR, Ex, LCtx,

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

SValBuilder::getDerivedRegionValueSymbolVal(SymbolRef parentSymbol,
 
   if (Loc::isLocType(T))
     return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
 
   return nonloc::SymbolVal(sym);
 }
 
 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
     // type, so return a function pointer for the method.
     // We don't need to play a similar trick for static member fields
     // because these are represented as plain VarDecls and not FieldDecls
     // in the AST.
     if (MD->isStatic())

clang/lib/StaticAnalyzer/Core/Store.cpp

Optional<const MemRegion *> StoreManager::castRegion(const MemRegion *R,
   if (CastToTy->isObjCObjectPointerType())
     return R->StripCasts();
 
   if (CastToTy->isBlockPointerType()) {
     // FIXME: We may need different solutions, depending on the symbol
     // 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
     // will be interpreted as UnknownVal.
     return None;
   }
 
   // Now assume we are casting from pointer to pointer. Other cases should

clang/lib/StaticAnalyzer/Core/SymbolManager.cpp

bool SymbolReaper::isLiveRegion(const MemRegion *MR) {
 
   if (const auto *VR = dyn_cast<VarRegion>(MR))
     return isLive(VR, true);
 
   // FIXME: This is a gross over-approximation. What we really need is a way to
   // 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) {
   if (TheLiving.count(sym)) {
     markDependentsLive(sym);
     return true;
   }