[analyzer][NFC] Move overconstrained check from reAssume to assumeDualImpl

[martong]

Summary

Depends on D126406. Checking of the overconstrained property is much
better suited here.

[steakhal]

LGTM; measure performance implications.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h

117–121

Shouldn't you remove this as well? Check the friends.

[martong]

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h

117–121

No, it is the ConstraintManager which checks the overconstrained property (in assumeDualImpl).

[martong]

In D126707#3549677, @steakhal wrote:

LGTM; measure performance implications.

Seems like there are no implications (or within the error margin).

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h

private:
   // solver re-evaluates the existing constraints and realizes the
   // contradiction `0 * 0 = 4`.
   // We also evaluate the negated constraint `x != 0`;  the constraint solver
   // deduces `x = 1` and then realizes the contradiction `1 * 1 = 4`.
   // Both children are infeasible, thus the parent state is marked as
   // posteriorly over-constrained. These parents are handled with special care:
   // we do not allow transitions to exploded nodes with such states.
   bool PosteriorlyOverconstrained = false;
   // Make internal constraint solver entities friends so they can access the
   // overconstrained-related functions. We want to keep this API inaccessible
   // for Checkers.
   friend class ConstraintManager;
-  friend ProgramStateRef reAssume(ProgramStateRef State,
-                                  const RangeSet *Constraint, SVal TheValue);
   bool isPosteriorlyOverconstrained() const {
     return PosteriorlyOverconstrained;
   }
   ProgramStateRef cloneAsPosteriorlyOverconstrained() const;
 
   unsigned refCount;
 
   /// makeWithStore - Return a ProgramState with the same values as the current
   ///  state with the exception of using the specified Store.

clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp

if (!P.first && P.second)
     return ConditionTruthVal(true);
   return {};
 }
 
 template <typename AssumeFunction>
 ConstraintManager::ProgramStatePair
 ConstraintManager::assumeDualImpl(ProgramStateRef &State,
                                   AssumeFunction &Assume) {
+  if (State->isPosteriorlyOverconstrained())
+    return {State, State};
+
   ProgramStateRef StTrue = Assume(true);
 
   if (!StTrue) {
     ProgramStateRef StFalse = Assume(false);
     if (LLVM_UNLIKELY(!StFalse)) { // both infeasible
       ProgramStateRef StInfeasible = State->cloneAsPosteriorlyOverconstrained();
       assert(StInfeasible->isPosteriorlyOverconstrained());
       // Checkers might rely on the API contract that both returned states

clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableSet.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <iterator>
 
 using namespace clang;
 using namespace ento;
 
EquivalenceClass::removeMember(ProgramStateRef State, const SymbolRef Old) {
   ClassMapTy Classes = State->get<ClassMap>();
   ClassMapTy::Factory &CMF = State->get_context<ClassMap>();
   Classes = CMF.remove(Classes, Old);
   State = State->set<ClassMap>(Classes);
 
   return State;
 }
 
-// We must declare reAssume in clang::ento, otherwise we could not declare that
-// as a friend in ProgramState. More precisely, the call of reAssume would be
-// ambiguous (one in the global namespace and an other which is declared in
-// ProgramState is in clang::ento).
-namespace clang {
-namespace ento {
 // Re-evaluate an SVal with top-level `State->assume` logic.
 LLVM_NODISCARD ProgramStateRef reAssume(ProgramStateRef State,
                                         const RangeSet *Constraint,
                                         SVal TheValue) {
-  assert(State);
-  if (State->isPosteriorlyOverconstrained())
-    return nullptr;
   if (!Constraint)
     return State;
 
   const auto DefinedVal = TheValue.castAs<DefinedSVal>();
 
   // If the SVal is 0, we can simply interpret that as `false`.
   if (Constraint->encodesFalseRange())
     return State->assume(DefinedVal, false);
 
   // If the constraint does not encode 0 then we can interpret that as `true`
   // AND as a Range(Set).
   if (Constraint->encodesTrueRange()) {
     State = State->assume(DefinedVal, true);
     if (!State)
       return nullptr;
     // Fall through, re-assume based on the range values as well.
   }
   // Overestimate the individual Ranges with the RangeSet' lowest and
   // highest values.
   return State->assumeInclusiveRange(DefinedVal, Constraint->getMinValue(),
                                      Constraint->getMaxValue(), true);
 }
-} // namespace ento
-} // namespace clang
 
 // Iterate over all symbols and try to simplify them. Once a symbol is
 // simplified then we check if we can merge the simplified symbol's equivalence
 // class to this class. This way, we simplify not just the symbols but the
 // classes as well: we strive to keep the number of the classes to be the
 // absolute minimum.
 LLVM_NODISCARD ProgramStateRef
 EquivalenceClass::simplify(SValBuilder &SVB, RangeSet::Factory &F,