[PATCH] D45557: [Analyzer] Fix for SValBuilder expressions rearrangement

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:443
   return Sym->getType() == Ty &&
+    APSIntType(Int) == BV.getAPSIntType(Sym->getType()) &&
     (!BinaryOperator::isComparisonOp(Op) ||
----------------
6. Therefore i conclude that this check should be moved to the branch around comment 3., and it'd become redundant here.


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:455-456
 
   // We expect everything to be of the same type - this type.
   QualType SingleTy;
 
----------------
1. We have expected `LSym` to be of this type.


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:469
       Opts.shouldAggressivelySimplifyRelationalComparison()) {
     SingleTy = LSym->getType();
     if (ResultTy != SVB.getConditionType())
----------------
2. It holds on this branch.


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:474
   } else if (BinaryOperator::isAdditiveOp(Op)) {
     SingleTy = ResultTy;
     // Substracting unsigned integers is a nightmare.
----------------
3. But not necessarily on that branch.


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:486
   SymbolRef RSym = Rhs.getAsSymbol();
   if (!RSym || RSym->getType() != SingleTy)
     return None;
----------------
4. We check that `RSym` is of the correct type, but `LSym` remains unchecked.


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:491
   llvm::APSInt LInt, RInt;
   std::tie(LSym, LInt) = decomposeSymbol(LSym, BV);
   std::tie(RSym, RInt) = decomposeSymbol(RSym, BV);
----------------
5. Type of `LInt` here is guaranteed to be equal to the type of `LSym`, because that's how we construct symbols. Which was supposed to also be equal to `SingleTy`, but we didn't check for that.


https://reviews.llvm.org/D45557