[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I think we'd rather delay the unsigned difference feature (as far as i understand, you don't rely on it in the iterator checker), than introduce the artificial cast to signed which may have unexpected consequences, because that's not how unsigned difference normally behaves. I'd think a bit more on this as well.

In https://reviews.llvm.org/D35109#806466, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D35109#806156, @NoQ wrote:
>
> > I think you might also need to convert `APSInt`s to an appropriate type, as done above. Type of right-hand-side `APSInt`s do not necessarily coincide with the type of the left-hand-side symbol or of the whole expression. `APSInt` operations crash when signedness doesn't match (and in a few other cases).
>
>
> Could you please help me by constructing an example for this scenario? I tried multiple options, but I always failed since I check the type of the two sides, as you proposed. So I need an example where `typeof(A+n) == typeof(B+m)` but `typeof(n) != typeof(m)`.


Hmm, you are right, there are only problems with casts of symbols, not of `APSInt`s; type of the latter is always equal to the type of the whole expression. So when two `SymIntExpr`s have the same type `T`, types of their right-hand sides are automatically equal to `T` as well. Please add this as a comment then :)

The rest looks good to me now.


https://reviews.llvm.org/D35109