baloghadamsoftware added inline comments.
================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:330
+ nonloc::ConcreteInt(Max), SVB.getConditionType());
+ if (auto DV = IsCappedFromAbove.getAs<DefinedSVal>()) {
+ if (State->assume(*DV, false))
----------------
george.karpenkov wrote:
> 6 lines of branching is probably better expressed as
>
> ```
> if (!isa<DefinedSVal>(IsCappedFromAbove) ||
> State->assume(*dyn_cast<DefinedSVal>(IsCappedFromAbove), false))
> return false
> ```
SVal is not a pointer, so isa<>() and dyn_cast<>() does not work here.
================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:376
+ // Fail to decompose: "reduce" the problem to the "$x + 0" case.
+ return std::make_tuple(Sym, BO_Add, BV.getValue(0, Sym->getType()));
+}
----------------
george.karpenkov wrote:
> Is it beneficial to do this though? At the point where `decomposeSymbol` is
> called we are still checking whether the rearrangement could be performed, so
> maybe just returning a false flag would be better?
Failing to decompose a symbol does not mean the rearrangement could not be
performed. If we have just A on the left side instead of A+m or A-m, then we
regard it as A+0.
================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:432
+ // "$x - $y" vs. "$y - $x" because those are solver's keys.
+ if (LInt > RInt) {
+ ResultSym = SymMgr.getSymSymExpr(RSym, BO_Sub, LSym, SymTy);
----------------
george.karpenkov wrote:
> I think this could be shortened and made more explicit by constructing the
> LHS and RHS first, and then reversing both and the comparison operator if RHS
> is negative.
Are you sure it would be shorter? Anyway, how to reverse a SymSymExpr?
https://reviews.llvm.org/D41938
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
