[PATCH] [analyzer][Bugfix/improvement] Fix for PR16833

[Aleksei Sidorin]

================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1691
@@ +1690,3 @@
+      defaultIsFeasible = false;
+      break;
+    }
----------------
jordan_rose wrote:
> I don't think this `break` is correct. Cases are not guaranteed to be in order.
Does the order really affect this code? The fact that DefaultSt is null means that all possible cases for switch were already covered (range-by-range), there is nothing left to consider and we can break out of loop. Or am I missed something?

================
Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:604-605
@@ +603,4 @@
+                                            const llvm::APSInt& Adjustment) {
+  ProgramStateRef St1 = assumeSymLT(state, sym, From, Adjustment);
+  ProgramStateRef St2 = assumeSymGT(state, sym, To, Adjustment);
+  RangeSet Range1 = St1 ? GetRange(St1, sym) : F.getEmptySet();
----------------
jordan_rose wrote:
> These are expensive operations. Is it possible to do this at a lower level? Or are there good reasons to do this extra work?
> 
> One reason I'm not so happy with this is that some day we could have constraints on one symbol affect another symbol, but this doesn't preserve that. Then again, neither would a lower-level solution.
Jordan, could you explain what do you mean here?
If you mean removing of creation of new states (creating ranges only), that could be done easily.
But it seems that you mean something else that I cannot understand correctly.
BTW, does the code in previous function need correction too?

http://reviews.llvm.org/D5102