[analyzer][NFC] Refactoring BugReporter.cpp P6.: Completely get rid of interestingness propagation

[Szelethus]

Summary

Do you remember the teaser in D65378?

When you think about this, it makes sense. If something is really important, we're tracking it anyways, and that system is sophisticated enough to mark actually interesting statements as such. I wouldn't say that it's even likely that subexpressions are also interesting (array[10 - x + x]), so I guess even if this produced any effects, its probably undesirable.

---

I originally started working on this refactoring effort to dig from the point where diagnostics construction starts, down to this part of the code. Turns out it doesn't do anything. In fact, after the analysis ends, only visitors may mark objects as interesting (which has any actual effect), which is AMAZING to know, because ConditionBRVisitor only has interestiness to work with when it constructs a diagnostics piece.

[NoQ]

I thereby confirm that i've no idea what was this code about.

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

 using StackDiagPair =
     std::pair<PathDiagnosticCallPiece *, const ExplodedNode *>;
 using StackDiagVector = SmallVector<StackDiagPair, 6>;
 
 /// Map from each node to the diagnostic pieces visitors emit for them.
 using VisitorsDiagnosticsTy =
     llvm::DenseMap<const ExplodedNode *, std::vector<PathDiagnosticPieceRef>>;
 
-using InterestingExprs = llvm::DenseSet<const Expr *>;
-
 /// A map from PathDiagnosticPiece to the LocationContext of the inlined
 /// function call it represents.
 using LocationContextMap =
     llvm::DenseMap<const PathPieces *, const LocationContext *>;
 
 /// A helper class that contains everything needed to construct a
 /// PathDiagnostic object. It does no much more then providing convenient
 /// getters and some well placed asserts for extra security.
class BugReportConstruct {
   LocationContextMap LCM;
   const SourceManager &SM;
 
 public:
   /// We keep stack of calls to functions as we're ascending the bug path.
   /// TODO: PathDiagnostic has a stack doing the same thing, shouldn't we use
   /// that instead?
   StackDiagVector CallStack;
-  InterestingExprs IE;
   /// The bug report we're constructing. For ease of use, this field is kept
   /// public, though some "shortcut" getters are provided for commonly used
   /// methods of PathDiagnostic.
   std::unique_ptr<PathDiagnostic> PD;
 
 public:
   BugReportConstruct(const PathDiagnosticConsumer *PDC,
                      const ExplodedNode *ErrorNode, const BugReport *R);
else
           std::make_shared<PathDiagnosticControlFlowPiece>(
               Start, End, "Taking true branch"));
 
     break;
   }
   }
 }
 
-// Cone-of-influence: support the reverse propagation of "interesting" symbols
-// and values by tracing interesting calculations backwards through evaluated
-// expressions along a path.  This is probably overly complicated, but the idea
-// is that if an expression computed an "interesting" value, the child
-// expressions are also likely to be "interesting" as well (which then
-// propagates to the values they in turn compute).  This reverse propagation
-// is needed to track interesting correlations across function call boundaries,
-// where formal arguments bind to actual arguments, etc.  This is also needed
-// because the constraint solver sometimes simplifies certain symbolic values
-// into constants when appropriate, and this complicates reasoning about
-// interesting values.
-
-static void reversePropagateIntererstingSymbols(BugReport &R,
-                                                InterestingExprs &IE,
-                                                const ProgramState *State,
-                                                const Expr *Ex,
-                                                const LocationContext *LCtx) {
-  SVal V = State->getSVal(Ex, LCtx);
-  if (!(R.isInteresting(V) || IE.count(Ex)))
-    return;
-
-  switch (Ex->getStmtClass()) {
-    default:
-      if (!isa<CastExpr>(Ex))
-        break;
-      LLVM_FALLTHROUGH;
-    case Stmt::BinaryOperatorClass:
-    case Stmt::UnaryOperatorClass: {
-      for (const Stmt *SubStmt : Ex->children()) {
-        if (const auto *child = dyn_cast_or_null<Expr>(SubStmt)) {
-          IE.insert(child);
-          SVal ChildV = State->getSVal(child, LCtx);
-          R.markInteresting(ChildV);
-        }
-      }
-      break;
-    }
-  }
-
-  R.markInteresting(V);
-}
-
-static void reversePropagateInterestingSymbols(BugReport &R,
-                                               InterestingExprs &IE,
-                                               const ProgramState *State,
-                                               const LocationContext *CalleeCtx)
-{
-  // FIXME: Handle non-CallExpr-based CallEvents.
-  const StackFrameContext *Callee = CalleeCtx->getStackFrame();
-  const Stmt *CallSite = Callee->getCallSite();
-  if (const auto *CE = dyn_cast_or_null<CallExpr>(CallSite)) {
-    if (const auto *FD = dyn_cast<FunctionDecl>(CalleeCtx->getDecl())) {
-      FunctionDecl::param_const_iterator PI = FD->param_begin(),
-                                         PE = FD->param_end();
-      CallExpr::const_arg_iterator AI = CE->arg_begin(), AE = CE->arg_end();
-      for (; AI != AE && PI != PE; ++AI, ++PI) {
-        if (const Expr *ArgE = *AI) {
-          if (const ParmVarDecl *PD = *PI) {
-            Loc LV = State->getLValue(PD, CalleeCtx);
-            if (R.isInteresting(LV) || R.isInteresting(State->getRawSVal(LV)))
-              IE.insert(ArgE);
-          }
-        }
-      }
-    }
-  }
-}
-
 //===----------------------------------------------------------------------===//
 // Functions for determining if a loop was executed 0 times.
 //===----------------------------------------------------------------------===//
 
 static bool isLoop(const Stmt *Term) {
   switch (Term->getStmtClass()) {
     case Stmt::ForStmtClass:
     case Stmt::WhileStmtClass:
if (Optional<CallExitEnd> CE = P.getAs<CallExitEnd>()) {
     auto Call = PathDiagnosticCallPiece::construct(*CE, SM);
     // Record the mapping from call piece to LocationContext.
     assert(!C.isInLocCtxMap(&Call->path) &&
            "We just entered a call, this must've been the first time we "
            "encounter its context!");
     C.updateLocCtxMap(&Call->path, CE->getCalleeContext());
 
     if (C.shouldAddPathEdges()) {
-      const Stmt *S = CE->getCalleeContext()->getCallSite();
-      // Propagate the interesting symbols accordingly.
-      if (const auto *Ex = dyn_cast_or_null<Expr>(S)) {
-        reversePropagateIntererstingSymbols(
-            *getBugReport(), C.IE, C.getCurrentNode()->getState().get(), Ex,
-            C.getCurrLocationContext());
-      }
       // Add the edge to the return site.
       addEdgeToPath(C.getActivePath(), PrevLoc, Call->callReturn);
       PrevLoc.invalidate();
     }
 
     auto *P = Call.get();
     C.getActivePath().push_front(std::move(Call));
 
     // Make the contents of the call the active path for now.
     C.PD->pushActivePath(&P->path);
     C.CallStack.push_back(StackDiagPair(P, C.getCurrentNode()));
     return;
   }
 
   if (auto PS = P.getAs<PostStmt>()) {
     if (!C.shouldAddPathEdges())
       return;
 
-    // For expressions, make sure we propagate the
-    // interesting symbols correctly.
-    if (const Expr *Ex = PS->getStmtAs<Expr>())
-      reversePropagateIntererstingSymbols(*getBugReport(), C.IE,
-                                          C.getCurrentNode()->getState().get(),
-                                          Ex, C.getCurrLocationContext());
-
     // Add an edge.  If this is an ObjCForCollectionStmt do
     // not add an edge here as it appears in the CFG both
     // as a terminator and as a terminator condition.
     if (!isa<ObjCForCollectionStmt>(PS->getStmt())) {
       PathDiagnosticLocation L =
           PathDiagnosticLocation(PS->getStmt(), SM, C.getCurrLocationContext());
       addEdgeToPath(C.getActivePath(), PrevLoc, L);
     }
 
   } else if (auto BE = P.getAs<BlockEdge>()) {
 
     if (!C.shouldAddPathEdges()) {
       generateMinimalDiagForBlockEdge(C, *BE);
       return;
     }
 
-    // Does this represent entering a call?  If so, look at propagating
-    // interesting symbols across call boundaries.
-    if (const ExplodedNode *NextNode = C.getCurrentNode()->getFirstPred()) {
-      const LocationContext *CallerCtx = NextNode->getLocationContext();
-      const LocationContext *CalleeCtx = C.getCurrLocationContext();
-      if (CallerCtx != CalleeCtx && C.shouldAddPathEdges()) {
-        reversePropagateInterestingSymbols(*getBugReport(), C.IE,
-                                           C.getCurrentNode()->getState().get(),
-                                           CalleeCtx);
-      }
-    }
-
     // Are we jumping to the head of a loop?  Add a special diagnostic.
     if (const Stmt *Loop = BE->getSrc()->getLoopTarget()) {
       PathDiagnosticLocation L(Loop, SM, C.getCurrLocationContext());
       const Stmt *Body = nullptr;
 
       if (const auto *FS = dyn_cast<ForStmt>(Loop))
         Body = FS->getBody();
       else if (const auto *WS = dyn_cast<WhileStmt>(Loop))