dcoughlin added a comment. The diagnostic text looks to me, but I do have a comment about the nested 'if' inline.
================ 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 \'" ---------------- 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 ..." } ``` https://reviews.llvm.org/D41816 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits