[analyzer] [NFC] Some miscellaneous clean ups and documentation fixes.

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h

virtual bool scanReachableSymbols(Store S, const MemRegion *R,
                                     ScanReachableSymbols &Visitor) = 0;
 
   virtual void print(Store store, raw_ostream &Out, const char* nl) = 0;
 
   class BindingsHandler {
   public:
     virtual ~BindingsHandler();
 
+    /// \return whether the iteration should continue.
     virtual bool HandleBinding(StoreManager& SMgr, Store store,
                                const MemRegion *region, SVal val) = 0;
   };
 
-  class FindUniqueBinding :
-  public BindingsHandler {
+  class FindUniqueBinding : public BindingsHandler {
     SymbolRef Sym;
     const MemRegion* Binding = nullptr;
     bool First = true;
 
   public:
     FindUniqueBinding(SymbolRef sym) : Sym(sym) {}
 
     explicit operator bool() { return First && Binding; }

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

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);
 }
 
+/// Write information about the type state change to {@code os},
+/// return whether the note should be generated.
+static bool shouldGenerateNote(llvm::raw_string_ostream &os,
+                               const RefVal *PrevT, const RefVal &CurrV,
+                               SmallVector<ArgEffect, 2> &AEffects) {
+  // Get the previous type state.
+  RefVal PrevV = *PrevT;
+
+  // Specially handle -dealloc.
+  if (std::find(AEffects.begin(), AEffects.end(), Dealloc) != AEffects.end()) {
+    // Determine if the object's reference count was pushed to zero.
+    assert(!PrevV.hasSameState(CurrV) && "The state should have changed.");
+    // We may not have transitioned to 'release' if we hit an error.
+    // This case is handled elsewhere.
+    if (CurrV.getKind() == RefVal::Released) {
+      assert(CurrV.getCombinedCounts() == 0);
+      os << "Object released by directly sending the '-dealloc' message";
+      return true;
+    }
+  }
+
+  // Determine if the typestate has changed.
+  if (!PrevV.hasSameState(CurrV))
+    switch (CurrV.getKind()) {
+    case RefVal::Owned:
+    case RefVal::NotOwned:
+      if (PrevV.getCount() == CurrV.getCount()) {
+        // Did an autorelease message get sent?
+        if (PrevV.getAutoreleaseCount() == CurrV.getAutoreleaseCount())
+          return false;
+
+        assert(PrevV.getAutoreleaseCount() < CurrV.getAutoreleaseCount());
+        os << "Object autoreleased";
+        return true;
+      }
+
+      if (PrevV.getCount() > CurrV.getCount())
+        os << "Reference count decremented.";
+      else
+        os << "Reference count incremented.";
+
+      if (unsigned Count = CurrV.getCount())
+        os << " The object now has a +" << Count << " retain count.";
+
+      return true;
+
+    case RefVal::Released:
+      if (CurrV.getIvarAccessHistory() ==
+              RefVal::IvarAccessHistory::ReleasedAfterDirectAccess &&
+          CurrV.getIvarAccessHistory() != PrevV.getIvarAccessHistory()) {
+        os << "Strong instance variable relinquished. ";
+      }
+      os << "Object released.";
+      return true;
+
+    case RefVal::ReturnedOwned:
+      // Autoreleases can be applied after marking a node ReturnedOwned.
+      if (CurrV.getAutoreleaseCount())
+        return false;
+
+      os << "Object returned to caller as an owning reference (single "
+            "retain count transferred to caller)";
+      return true;
+
+    case RefVal::ReturnedNotOwned:
+      os << "Object returned to caller with a +0 retain count";
+      return true;
+
+    default:
+      return false;
+    }
+  return true;
+}
+
 std::shared_ptr<PathDiagnosticPiece>
 CFRefReportVisitor::VisitNode(const ExplodedNode *N,
                               BugReporterContext &BRC, BugReport &BR) {
   // FIXME: We will eventually need to handle non-statement-based events
   // (__attribute__((cleanup))).
   if (!N->getLocation().getAs<StmtPoint>())
     return nullptr;
 
if (!PrevT) {
 
     if (isa<ObjCIvarRefExpr>(S) &&
         isSynthesizedAccessor(LCtx->getStackFrame())) {
       S = LCtx->getStackFrame()->getCallSite();
     }
 
     if (isa<ObjCArrayLiteral>(S)) {
       os << "NSArray literal is an object with a +0 retain count";
-    }
-    else if (isa<ObjCDictionaryLiteral>(S)) {
+    } else if (isa<ObjCDictionaryLiteral>(S)) {
       os << "NSDictionary literal is an object with a +0 retain count";
-    }
-    else if (const ObjCBoxedExpr *BL = dyn_cast<ObjCBoxedExpr>(S)) {
+    } else if (const ObjCBoxedExpr *BL = dyn_cast<ObjCBoxedExpr>(S)) {
       if (isNumericLiteralExpression(BL->getSubExpr()))
         os << "NSNumber literal is an object with a +0 retain count";
       else {
         const ObjCInterfaceDecl *BoxClass = nullptr;
         if (const ObjCMethodDecl *Method = BL->getBoxingMethod())
           BoxClass = Method->getClassInterface();
 
         // We should always be able to find the boxing class interface,
         // but consider this future-proofing.
-        if (BoxClass)
+        if (BoxClass) {
           os << *BoxClass << " b";
-        else
+        } else {
           os << "B";
+        }
 
         os << "oxed expression produces an object with a +0 retain count";
       }
-    }
-    else if (isa<ObjCIvarRefExpr>(S)) {
+    } else if (isa<ObjCIvarRefExpr>(S)) {
       os << "Object loaded from instance variable";
-    }
-    else {
+    } else {
       if (const CallExpr *CE = dyn_cast<CallExpr>(S)) {
         // Get the name of the callee (if it is available).
         SVal X = CurrSt->getSValAsScalarOrLoc(CE->getCallee(), LCtx);
-        if (const FunctionDecl *FD = X.getAsFunctionDecl())
+        if (const FunctionDecl *FD = X.getAsFunctionDecl()) {
           os << "Call to function '" << *FD << '\'';
-        else
+        } else {
           os << "function call";
         }
-      else {
+      } else {
         assert(isa<ObjCMessageExpr>(S));
         CallEventManager &Mgr = CurrSt->getStateManager().getCallEventManager();
         CallEventRef<ObjCMethodCall> Call
           = Mgr.getObjCMethodCall(cast<ObjCMessageExpr>(S), CurrSt, LCtx);
 
         switch (Call->getMessageKind()) {
         case OCM_Message:
           os << "Method";
if (const RetainSummary *Summ = SummaryLog.lookup(OrigNode)) {
     // ObjCMessageExprs.
     const Stmt *S = N->getLocation().castAs<StmtPoint>().getStmt();
 
     if (const CallExpr *CE = dyn_cast<CallExpr>(S)) {
       // Iterate through the parameter expressions and see if the symbol
       // was ever passed as an argument.
       unsigned i = 0;
 
-      for (CallExpr::const_arg_iterator AI=CE->arg_begin(), AE=CE->arg_end();
-           AI!=AE; ++AI, ++i) {
+      for (auto AI=CE->arg_begin(), AE=CE->arg_end(); AI!=AE; ++AI, ++i) {
 
         // Retrieve the value of the argument.  Is it the symbol
         // we are interested in?
         if (CurrSt->getSValAsScalarOrLoc(*AI, LCtx).getAsLocSymbol() != Sym)
           continue;
 
         // We have an argument.  Get the effect!
         AEffects.push_back(Summ->getArg(i));
       }
     } else if (const ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S)) {
       if (const Expr *receiver = ME->getInstanceReceiver()) {
         if (CurrSt->getSValAsScalarOrLoc(receiver, LCtx)
               .getAsLocSymbol() == Sym) {
           // The symbol we are tracking is the receiver.
           AEffects.push_back(Summ->getReceiverEffect());
         }
       }
     }
   }
 
-  do {
-    // Get the previous type state.
-    RefVal PrevV = *PrevT;
-
-    // Specially handle -dealloc.
-    if (std::find(AEffects.begin(), AEffects.end(), Dealloc) !=
-                          AEffects.end()) {
-      // Determine if the object's reference count was pushed to zero.
-      assert(!PrevV.hasSameState(CurrV) && "The state should have changed.");
-      // We may not have transitioned to 'release' if we hit an error.
-      // This case is handled elsewhere.
-      if (CurrV.getKind() == RefVal::Released) {
-        assert(CurrV.getCombinedCounts() == 0);
-        os << "Object released by directly sending the '-dealloc' message";
-        break;
-      }
-    }
-
-    // Determine if the typestate has changed.
-    if (!PrevV.hasSameState(CurrV))
-      switch (CurrV.getKind()) {
-        case RefVal::Owned:
-        case RefVal::NotOwned:
-          if (PrevV.getCount() == CurrV.getCount()) {
-            // Did an autorelease message get sent?
-            if (PrevV.getAutoreleaseCount() == CurrV.getAutoreleaseCount())
-              return nullptr;
+  if (!shouldGenerateNote(os, PrevT, CurrV, AEffects))
 
-            assert(PrevV.getAutoreleaseCount() < CurrV.getAutoreleaseCount());
-            os << "Object autoreleased";
-            break;
-          }
-
-          if (PrevV.getCount() > CurrV.getCount())
-            os << "Reference count decremented.";
-          else
-            os << "Reference count incremented.";
-
-          if (unsigned Count = CurrV.getCount())
-            os << " The object now has a +" << Count << " retain count.";
-
-          break;
-
-        case RefVal::Released:
-          if (CurrV.getIvarAccessHistory() ==
-                RefVal::IvarAccessHistory::ReleasedAfterDirectAccess &&
-              CurrV.getIvarAccessHistory() != PrevV.getIvarAccessHistory()) {
-            os << "Strong instance variable relinquished. ";
-          }
-          os << "Object released.";
-          break;
-
-        case RefVal::ReturnedOwned:
-          // Autoreleases can be applied after marking a node ReturnedOwned.
-          if (CurrV.getAutoreleaseCount())
-            return nullptr;
-
-          os << "Object returned to caller as an owning reference (single "
-                "retain count transferred to caller)";
-          break;
-
-        case RefVal::ReturnedNotOwned:
-          os << "Object returned to caller with a +0 retain count";
-          break;
-
-        default:
-          return nullptr;
-      }
-  } while (0);
-
   if (os.str().empty())
     return nullptr; // We have nothing to say!
 
   const Stmt *S = N->getLocation().castAs<StmtPoint>().getStmt();
   PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
                                 N->getLocationContext());
   auto P = std::make_shared<PathDiagnosticEventPiece>(Pos, os.str());
 
CFRefLeakReportVisitor::getEndPath(BugReporterContext &BRC,
   std::string sbuf;
   llvm::raw_string_ostream os(sbuf);
 
   os << "Object leaked: ";
 
   Optional<std::string> RegionDescription = describeRegion(FirstBinding);
   if (RegionDescription) {
     os << "object allocated and stored into '" << *RegionDescription << '\'';
-  }
-  else
+  } else {
     os << "allocated object";
+  }
 
   // Get the retain count.
   const RefVal* RV = getRefBinding(EndN->getState(), Sym);
   assert(RV);
 
   if (RV->getKind() == RefVal::ErrorLeakReturned) {
     // FIXME: Per comments in rdar://6320065, "create" only applies to CF
     // objects.  Only "copy", "alloc", "retain" and "new" transfer ownership