[analyzer] ConditionBRVisitor: Remove duplicated code

[Charusso]

Summary

-

[Charusso]

• return nullptr if no value print happened.

[george.karpenkov]

include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

216 ↗

(On Diff #186733)

Please add a doxygen comment describing semantics and return value.

[Charusso]

• Added doxygen comment.

[Charusso]

...properly.

[NoQ]

Looks great, thanks!

[Charusso]

• Rebased

[Charusso]

• Rebased

cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h

VisitTrueTest(const Expr *Cond, const BinaryOperator *BExpr,
                 BugReporterContext &BRC, BugReport &R, const ExplodedNode *N,
                 bool TookTrue, bool IsAssuming);
 
   std::shared_ptr<PathDiagnosticPiece>
   VisitConditionVariable(StringRef LhsString, const Expr *CondVarExpr,
                          BugReporterContext &BRC, BugReport &R,
                          const ExplodedNode *N, bool TookTrue);
 
+  /// Tries to print the value of the given expression.
+  ///
+  /// \param CondVarExpr The expression to print its value.
+  /// \param Out The stream to print.
+  /// \param N The node where we encountered the condition.
+  /// \param TookTrue Whether we took the \c true branch of the condition.
+  ///
+  /// \return Whether the print was successful. (The printing is successful if
+  ///         we model the value and we could obtain it.)
+  bool printValue(const Expr *CondVarExpr, raw_ostream &Out,
+                  const ExplodedNode *N, bool TookTrue, bool IsAssuming);
+
   bool patternMatch(const Expr *Ex,
                     const Expr *ParentEx,
                     raw_ostream &Out,
                     BugReporterContext &BRC,
                     BugReport &R,
                     const ExplodedNode *N,
                     Optional<bool> &prunable);
 

cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

std::shared_ptr<PathDiagnosticPiece> ConditionBRVisitor::VisitConditionVariable(
     BugReport &report, const ExplodedNode *N, bool TookTrue) {
   // FIXME: If there's already a constraint tracker for this variable,
   // we shouldn't emit anything here (c.f. the double note in
   // test/Analysis/inlining/path-notes.c)
   SmallString<256> buf;
   llvm::raw_svector_ostream Out(buf);
   Out << "Assuming " << LhsString << " is ";
 
-  QualType Ty = CondVarExpr->getType();
-
-  if (Ty->isPointerType())
-    Out << (TookTrue ? "not null" : "null");
-  else if (Ty->isObjCObjectPointerType())
-    Out << (TookTrue ? "not nil" : "nil");
-  else if (Ty->isBooleanType())
-    Out << (TookTrue ? "true" : "false");
-  else if (Ty->isIntegralOrEnumerationType())
-    Out << (TookTrue ? "non-zero" : "zero");
-  else
+  if (!printValue(CondVarExpr, Out, N, TookTrue, /*IsAssuming=*/true))
     return nullptr;
 
   const LocationContext *LCtx = N->getLocationContext();
   PathDiagnosticLocation Loc(CondVarExpr, BRC.getSourceManager(), LCtx);
   auto event = std::make_shared<PathDiagnosticEventPiece>(Loc, Out.str());
 
   if (const auto *DR = dyn_cast<DeclRefExpr>(CondVarExpr)) {
     if (const auto *VD = dyn_cast<VarDecl>(DR->getDecl())) {
std::shared_ptr<PathDiagnosticPiece> ConditionBRVisitor::VisitTrueTest(
   if (!VD)
     return nullptr;
 
   SmallString<256> Buf;
   llvm::raw_svector_ostream Out(Buf);
 
   Out << (IsAssuming ? "Assuming '" : "'") << VD->getDeclName() << "' is ";
 
-  QualType Ty = VD->getType();
-
-  if (Ty->isPointerType())
-    Out << (TookTrue ? "non-null" : "null");
-  else if (Ty->isObjCObjectPointerType())
-    Out << (TookTrue ? "non-nil" : "nil");
-  else if (Ty->isScalarType()) {
-    Optional<const llvm::APSInt *> IntValue;
-    if (!IsAssuming)
-      IntValue = getConcreteIntegerValue(DRE, N);
-
-    if (IsAssuming || !IntValue.hasValue())
-      Out << (TookTrue ? "not equal to 0" : "0");
-    else
-      Out << *IntValue.getValue();
-  } else
+  if (!printValue(DRE, Out, N, TookTrue, IsAssuming))
     return nullptr;
 
   const LocationContext *LCtx = N->getLocationContext();
   PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
 
   // If we know the value create a pop-up note.
   if (!IsAssuming)
     return std::make_shared<PathDiagnosticPopUpPiece>(Loc, Out.str());
 
   auto event = std::make_shared<PathDiagnosticEventPiece>(Loc, Out.str());
   const ProgramState *state = N->getState().get();
   if (const MemRegion *R = state->getLValue(VD, LCtx).getAsRegion()) {
     if (report.isInteresting(R))
       event->setPrunable(false);
     else {
       SVal V = state->getSVal(R);
       if (report.isInteresting(V))
         event->setPrunable(false);
     }
   }
   return std::move(event);
 }
 
+bool ConditionBRVisitor::printValue(const Expr *CondVarExpr, raw_ostream &Out,
+                                    const ExplodedNode *N, bool TookTrue,
+                                    bool IsAssuming) {
+  QualType Ty = CondVarExpr->getType();
+
+  if (Ty->isPointerType()) {
+    Out << (TookTrue ? "non-null" : "null");
+    return true;
+  }
+
+  if (Ty->isObjCObjectPointerType()) {
+    Out << (TookTrue ? "non-nil" : "nil");
+    return true;
+  }
+
+  if (!Ty->isIntegralOrEnumerationType())
+    return false;
+
+  Optional<const llvm::APSInt *> IntValue;
+  if (!IsAssuming)
+    IntValue = getConcreteIntegerValue(CondVarExpr, N);
+
+  if (IsAssuming || !IntValue.hasValue())
+    Out << (TookTrue ? "not equal to 0" : "0");
+  else
+    Out << *IntValue.getValue();
+
+  return true;
+}
+
 const char *const ConditionBRVisitor::GenericTrueMessage =
     "Assuming the condition is true";
 const char *const ConditionBRVisitor::GenericFalseMessage =
     "Assuming the condition is false";
 
 bool ConditionBRVisitor::isPieceMessageGeneric(
     const PathDiagnosticPiece *Piece) {
   return Piece->getString() == GenericTrueMessage ||