rnkovacs added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:150
+ SB.getKnownValue(state, C.getSVal(B->getRHS()));
+ if ((unsigned) RHS->getZExtValue() > LHS->countLeadingZeros()) {
+ OS << "The result of the left shift is undefined due to shifting \'"
----------------
dcoughlin wrote:
> This inner 'if' looks fishy to me because if the 'else' branch is ever taken
> then OS will be empty.
>
> If the else branch can't be taken then you should turn it into an assert().
> If it can be taken, then you should make sure that the fall through goes
> through the "default" else case at the bottom. One way to do this is to pull
> out the "is representable logic" into a helper function and call that in the
> containing 'else if'.
>
> Something like:
>
> ```
> if (B->getOpcode() == BinaryOperatorKind::BO_Shl &&
> isLeftShiftResultRepresentable(LHS, RHS)) {
> OS << "The result of the left shift ..."
> }
> ```
I overlooked this issue, thanks for pointing out. I pulled the logic out into a
helper function.
https://reviews.llvm.org/D41816
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
