[analyzer] [NFC] Eliminate dispatchCast, evalCastFromNonLoc and evalCastFromLoc functions from SValBuilder

[ASDenysPetrov]

Summary

Remove dispatchCast, evalCastFromNonLoc and evalCastFromLoc functions since their functionality has been moved to common evalCast function.
Use evalCast instead.

Post-clean up patch for D96090 patch.
The patch shall not change any behavior.

[ASDenysPetrov]

Rebased on main.

[vsavchenko]

Looks great, thanks for cleaning up!

[steakhal]

Please, @ASDenysPetrov, mention NFC in the commit message when you commit this change.
The same applies to all of your NFC changes.

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

protected:
   ProgramStateManager &StateMgr;
 
   /// The scalar type to use for array indices.
   const QualType ArrayIndexTy;
 
   /// The width of the scalar type used for array indices.
   const unsigned ArrayIndexWidth;
 
-  virtual SVal evalCastFromNonLoc(NonLoc val, QualType castTy) = 0;
-  virtual SVal evalCastFromLoc(Loc val, QualType castTy) = 0;
-
   SVal evalCastKind(UndefinedVal V, QualType CastTy, QualType OriginalTy);
   SVal evalCastKind(UnknownVal V, QualType CastTy, QualType OriginalTy);
   SVal evalCastKind(Loc V, QualType CastTy, QualType OriginalTy);
   SVal evalCastKind(NonLoc V, QualType CastTy, QualType OriginalTy);
   SVal evalCastSubKind(loc::ConcreteInt V, QualType CastTy,
                        QualType OriginalTy);
   SVal evalCastSubKind(loc::GotoLabel V, QualType CastTy, QualType OriginalTy);
   SVal evalCastSubKind(loc::MemRegionVal V, QualType CastTy,
                        QualType OriginalTy);
   SVal evalCastSubKind(nonloc::CompoundVal V, QualType CastTy,
                        QualType OriginalTy);
   SVal evalCastSubKind(nonloc::ConcreteInt V, QualType CastTy,
                        QualType OriginalTy);
   SVal evalCastSubKind(nonloc::LazyCompoundVal V, QualType CastTy,
                        QualType OriginalTy);
   SVal evalCastSubKind(nonloc::LocAsInteger V, QualType CastTy,
                        QualType OriginalTy);
   SVal evalCastSubKind(nonloc::SymbolVal V, QualType CastTy,
                        QualType OriginalTy);
   SVal evalCastSubKind(nonloc::PointerToMember V, QualType CastTy,
                        QualType OriginalTy);
 
 public:
-  // FIXME: Make these protected again once RegionStoreManager correctly
-  // handles loads from different bound value types.
-  virtual SVal dispatchCast(SVal val, QualType castTy) = 0;
-
-public:
   SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
               ProgramStateManager &stateMgr)
       : Context(context), BasicVals(context, alloc),
         SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
         StateMgr(stateMgr), ArrayIndexTy(context.LongLongTy),
         ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
 
   virtual ~SValBuilder() = default;

clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

SVal SValBuilder::convertToArrayIndex(SVal val) {
 
   // Common case: we have an appropriately sized integer.
   if (Optional<nonloc::ConcreteInt> CI = val.getAs<nonloc::ConcreteInt>()) {
     const llvm::APSInt& I = CI->getValue();
     if (I.getBitWidth() == ArrayIndexWidth && I.isSigned())
       return val;
   }
 
-  return evalCastFromNonLoc(val.castAs<NonLoc>(), ArrayIndexTy);
+  return evalCast(val, ArrayIndexTy);
 }
 
 nonloc::ConcreteInt SValBuilder::makeBoolVal(const CXXBoolLiteralExpr *boolean){
   return makeTruthVal(boolean->getValue());
 }
 
 DefinedOrUnknownSVal
 SValBuilder::getRegionValueSymbolVal(const TypedValueRegion *region) {

clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 class SimpleSValBuilder : public SValBuilder {
-protected:
-  SVal dispatchCast(SVal val, QualType castTy) override;
-  SVal evalCastFromNonLoc(NonLoc val, QualType castTy) override;
-  SVal evalCastFromLoc(Loc val, QualType castTy) override;
-
 public:
   SimpleSValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
                     ProgramStateManager &stateMgr)
                     : SValBuilder(alloc, context, stateMgr) {}
   ~SimpleSValBuilder() override {}
 
   SVal evalMinus(NonLoc val) override;
   SVal evalComplement(NonLoc val) override;

 
 SValBuilder *ento::createSimpleSValBuilder(llvm::BumpPtrAllocator &alloc,
                                            ASTContext &context,
                                            ProgramStateManager &stateMgr) {
   return new SimpleSValBuilder(alloc, context, stateMgr);
 }
 
 //===----------------------------------------------------------------------===//
-// Transfer function for Casts.
-//===----------------------------------------------------------------------===//
-
-// FIXME: This function should be eliminated and replaced with `evalCast`
-SVal SimpleSValBuilder::dispatchCast(SVal Val, QualType CastTy) {
-  return evalCast(Val, CastTy);
-}
-
-// FIXME: This function should be eliminated and replaced with `evalCast`
-SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
-  bool isLocType = Loc::isLocType(castTy);
-  if (val.getAs<nonloc::PointerToMember>())
-    return val;
-
-  if (Optional<nonloc::LocAsInteger> LI = val.getAs<nonloc::LocAsInteger>()) {
-    if (isLocType)
-      return LI->getLoc();
-    // FIXME: Correctly support promotions/truncations.
-    unsigned castSize = Context.getIntWidth(castTy);
-    if (castSize == LI->getNumBits())
-      return val;
-    return makeLocAsInteger(LI->getLoc(), castSize);
-  }
-
-  if (SymbolRef se = val.getAsSymbol()) {
-    QualType T = Context.getCanonicalType(se->getType());
-    // If types are the same or both are integers, ignore the cast.
-    // FIXME: Remove this hack when we support symbolic truncation/extension.
-    // HACK: If both castTy and T are integers, ignore the cast.  This is
-    // not a permanent solution.  Eventually we want to precisely handle
-    // extension/truncation of symbolic integers.  This prevents us from losing
-    // precision when we assign 'x = y' and 'y' is symbolic and x and y are
-    // different integer types.
-   if (haveSameType(T, castTy))
-      return val;
-
-    if (!isLocType)
-      return makeNonLoc(se, T, castTy);
-    return UnknownVal();
-  }
-
-  // If value is a non-integer constant, produce unknown.
-  if (!val.getAs<nonloc::ConcreteInt>())
-    return UnknownVal();
-
-  // Handle casts to a boolean type.
-  if (castTy->isBooleanType()) {
-    bool b = val.castAs<nonloc::ConcreteInt>().getValue().getBoolValue();
-    return makeTruthVal(b, castTy);
-  }
-
-  // Only handle casts from integers to integers - if val is an integer constant
-  // being cast to a non-integer type, produce unknown.
-  if (!isLocType && !castTy->isIntegralOrEnumerationType())
-    return UnknownVal();
-
-  llvm::APSInt i = val.castAs<nonloc::ConcreteInt>().getValue();
-  BasicVals.getAPSIntType(castTy).apply(i);
-
-  if (isLocType)
-    return makeIntLocVal(i);
-  else
-    return makeIntVal(i);
-}
-
-// FIXME: This function should be eliminated and replaced with `evalCast`
-SVal SimpleSValBuilder::evalCastFromLoc(Loc val, QualType castTy) {
-
-  // Casts from pointers -> pointers, just return the lval.
-  //
-  // Casts from pointers -> references, just return the lval.  These
-  //   can be introduced by the frontend for corner cases, e.g
-  //   casting from va_list* to __builtin_va_list&.
-  //
-  if (Loc::isLocType(castTy) || castTy->isReferenceType())
-    return val;
-
-  // FIXME: Handle transparent unions where a value can be "transparently"
-  //  lifted into a union type.
-  if (castTy->isUnionType())
-    return UnknownVal();
-
-  // Casting a Loc to a bool will almost always be true,
-  // unless this is a weak function or a symbolic region.
-  if (castTy->isBooleanType()) {
-    switch (val.getSubKind()) {
-      case loc::MemRegionValKind: {
-        const MemRegion *R = val.castAs<loc::MemRegionVal>().getRegion();
-        if (const FunctionCodeRegion *FTR = dyn_cast<FunctionCodeRegion>(R))
-          if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(FTR->getDecl()))
-            if (FD->isWeak())
-              // FIXME: Currently we are using an extent symbol here,
-              // because there are no generic region address metadata
-              // symbols to use, only content metadata.
-              return nonloc::SymbolVal(SymMgr.getExtentSymbol(FTR));
-
-        if (const SymbolicRegion *SymR = R->getSymbolicBase())
-          return makeNonLoc(SymR->getSymbol(), BO_NE,
-                            BasicVals.getZeroWithPtrWidth(), castTy);
-
-        // FALL-THROUGH
-        LLVM_FALLTHROUGH;
-      }
-
-      case loc::GotoLabelKind:
-        // Labels and non-symbolic memory regions are always true.
-        return makeTruthVal(true, castTy);
-    }
-  }
-
-  if (castTy->isIntegralOrEnumerationType()) {
-    unsigned BitWidth = Context.getIntWidth(castTy);
-
-    if (!val.getAs<loc::ConcreteInt>())
-      return makeLocAsInteger(val, BitWidth);
-
-    llvm::APSInt i = val.castAs<loc::ConcreteInt>().getValue();
-    BasicVals.getAPSIntType(castTy).apply(i);
-    return makeIntVal(i);
-  }
-
-  // All other cases: return 'UnknownVal'.  This includes casting pointers
-  // to floats, which is probably badness it itself, but this is a good
-  // intermediate solution until we do something better.
-  return UnknownVal();
-}
-
-//===----------------------------------------------------------------------===//
 // Transfer function for unary operators.
 //===----------------------------------------------------------------------===//
 
 SVal SimpleSValBuilder::evalMinus(NonLoc val) {
   switch (val.getSubKind()) {
   case nonloc::ConcreteIntKind:
     return val.castAs<nonloc::ConcreteInt>().evalMinus(*this);
   default:
case BO_Or:
     else if (RHS.isAllOnesValue()) {
       const llvm::APSInt &Result = BasicVals.Convert(resultTy, RHS);
       return nonloc::ConcreteInt(Result);
     }
     break;
   }
 
   // Idempotent ops (like a*1) can still change the type of an expression.
-  // Wrap the LHS up in a NonLoc again and let evalCastFromNonLoc do the
+  // Wrap the LHS up in a NonLoc again and let evalCast do the
   // dirty work.
   if (isIdempotent)
-      return evalCastFromNonLoc(nonloc::SymbolVal(LHS), resultTy);
+    return evalCast(nonloc::SymbolVal(LHS), resultTy);
 
   // If we reach this point, the expression cannot be simplified.
   // Make a SymbolVal for the entire expression, after converting the RHS.
   const llvm::APSInt *ConvertedRHS = &RHS;
   if (BinaryOperator::isComparisonOp(op)) {
     // We're looking for a type big enough to compare the symbolic value
     // with the given constant.
     // FIXME: This is an approximation of Sema::UsualArithmeticConversions.
switch (op) {
       case BO_LT:
       case BO_GT:
       case BO_NE:
         return makeTruthVal(false, resultTy);
       case BO_Xor:
       case BO_Sub:
         if (resultTy->isIntegralOrEnumerationType())
           return makeIntVal(0, resultTy);
-        return evalCastFromNonLoc(makeIntVal(0, /*isUnsigned=*/false), resultTy);
+        return evalCast(makeIntVal(0, /*isUnsigned=*/false), resultTy);
       case BO_Or:
       case BO_And:
-        return evalCastFromNonLoc(lhs, resultTy);
+        return evalCast(lhs, resultTy);
     }
 
   while (1) {
     switch (lhs.getSubKind()) {
     default:
       return makeSymExprValNN(op, lhs, rhs, resultTy);
     case nonloc::PointerToMemberKind: {
       assert(rhs.getSubKind() == nonloc::PointerToMemberKind &&
case nonloc::ConcreteIntKind: {
       case BO_And:
       case BO_Xor:
       case BO_Or:
         std::swap(lhs, rhs);
         continue;
       case BO_Shr:
         // (~0)>>a
         if (LHSValue.isAllOnesValue() && LHSValue.isSigned())
-          return evalCastFromNonLoc(lhs, resultTy);
+          return evalCast(lhs, resultTy);
         LLVM_FALLTHROUGH;
       case BO_Shl:
         // 0<<a and 0>>a
         if (LHSValue == 0)
-          return evalCastFromNonLoc(lhs, resultTy);
+          return evalCast(lhs, resultTy);
         return makeSymExprValNN(op, InputLHS, InputRHS, resultTy);
       case BO_Rem:
         // 0 % x == 0
         if (LHSValue == 0)
           return makeZeroVal(resultTy);
         LLVM_FALLTHROUGH;
       default:
         return makeSymExprValNN(op, InputLHS, InputRHS, resultTy);
SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
 
   case loc::GotoLabelKind:
     // The only thing we know about labels is that they're non-null.
     if (rhs.isZeroConstant()) {
       switch (op) {
       default:
         break;
       case BO_Sub:
-        return evalCastFromLoc(lhs, resultTy);
+        return evalCast(lhs, resultTy);
       case BO_EQ:
       case BO_LE:
       case BO_LT:
         return makeTruthVal(false, resultTy);
       case BO_NE:
       case BO_GT:
       case BO_GE:
         return makeTruthVal(true, resultTy);
if (SymbolRef rSym = rhs.getAsLocSymbol()) {
       return makeNonLoc(rSym, op, lVal, resultTy);
     }
 
     // If both operands are constants, just perform the operation.
     if (Optional<loc::ConcreteInt> rInt = rhs.getAs<loc::ConcreteInt>()) {
       SVal ResultVal =
           lhs.castAs<loc::ConcreteInt>().evalBinOp(BasicVals, op, *rInt);
       if (Optional<NonLoc> Result = ResultVal.getAs<NonLoc>())
-        return evalCastFromNonLoc(*Result, resultTy);
+        return evalCast(*Result, resultTy);
 
       assert(!ResultVal.getAs<Loc>() && "Loc-Loc ops should not produce Locs");
       return UnknownVal();
     }
 
     // Special case comparisons against NULL.
     // This must come after the test if the RHS is a symbol, which is used to
     // build constraints. The address of any non-symbolic region is guaranteed
if (Optional<loc::ConcreteInt> rInt = rhs.getAs<loc::ConcreteInt>()) {
         return UnknownVal();
       }
       // Special case comparisons to NULL.
       // This must come after the test if the LHS is a symbol, which is used to
       // build constraints. The address of any non-symbolic region is guaranteed
       // to be non-NULL.
       if (rInt->isZeroConstant()) {
         if (op == BO_Sub)
-          return evalCastFromLoc(lhs, resultTy);
+          return evalCast(lhs, resultTy);
 
         if (BinaryOperator::isComparisonOp(op)) {
           QualType boolType = getContext().BoolTy;
-          NonLoc l = evalCastFromLoc(lhs, boolType).castAs<NonLoc>();
+          NonLoc l = evalCast(lhs, boolType).castAs<NonLoc>();
           NonLoc r = makeTruthVal(false, boolType).castAs<NonLoc>();
           return evalBinOpNN(state, op, l, r, resultTy);
         }
       }
 
       // Comparing a region to an arbitrary integer is completely unknowable.
       return UnknownVal();
     }
if (RightER && LeftER) {
       if (LeftER->getSuperRegion() == RightER->getSuperRegion() &&
           LeftER->getElementType() == RightER->getElementType()) {
         // Get the left index and cast it to the correct type.
         // If the index is unknown or undefined, bail out here.
         SVal LeftIndexVal = LeftER->getIndex();
         Optional<NonLoc> LeftIndex = LeftIndexVal.getAs<NonLoc>();
         if (!LeftIndex)
           return UnknownVal();
-        LeftIndexVal = evalCastFromNonLoc(*LeftIndex, ArrayIndexTy);
+        LeftIndexVal = evalCast(*LeftIndex, ArrayIndexTy);
         LeftIndex = LeftIndexVal.getAs<NonLoc>();
         if (!LeftIndex)
           return UnknownVal();
 
         // Do the same for the right index.
         SVal RightIndexVal = RightER->getIndex();
         Optional<NonLoc> RightIndex = RightIndexVal.getAs<NonLoc>();
         if (!RightIndex)
           return UnknownVal();
-        RightIndexVal = evalCastFromNonLoc(*RightIndex, ArrayIndexTy);
+        RightIndexVal = evalCast(*RightIndex, ArrayIndexTy);
         RightIndex = RightIndexVal.getAs<NonLoc>();
         if (!RightIndex)
           return UnknownVal();
 
         // Actually perform the operation.
         // evalBinOpNN expects the two indexes to already be the right type.
         return evalBinOpNN(state, op, *LeftIndex, *RightIndex, resultTy);
       }