lebedev.ri added inline comments.
================ Comment at: docs/UndefinedBehaviorSanitizer.rst:134 + integer promotions, as those may result in an unexpected computation + results, even though no overflow happens (signed or unsigned). - ``-fsanitize=unreachable``: If control flow reaches an unreachable ---------------- vsk wrote: > Could you make this more explicit? It would help to point out that this check > does not diagnose lossy implicit integer conversions, but that the new check > does. Ditto for the comment in the unsigned-integer-overflow section. Is this better? ================ Comment at: lib/CodeGen/CodeGenFunction.h:383 + // This stack is used/maintained exclusively by the implicit cast sanitizer. + llvm::SmallVector<const CastExpr *, 8> CastExprStack; + ---------------- vsk wrote: > Why not 0 instead of 8, given that in the common case, this stack is unused? No longer relevant. ================ Comment at: lib/CodeGen/CodeGenFunction.h:388 + + llvm::SmallVector<const CastExpr *, 2> GuardedCasts; + ---------------- vsk wrote: > I'm not sure the cost of maintaining an extra vector is worth the benefit of > the added assertion. Wouldn't it be cheaper to just store the number of > pushed casts? You'd only need one constructor which accepts an ArrayRef<const > CastExpr *>. No longer relevant. ================ Comment at: test/CodeGen/catch-implicit-integer-truncations.c:29 + // CHECK-SANITIZE-NEXT: %[[TRUNCHECK:.*]] = icmp eq i32 %[[ANYEXT]], %[[SRC]], !nosanitize + // CHECK-SANITIZE-ANYRECOVER-NEXT: br i1 %[[TRUNCHECK]], label %[[CONT:.*]], label %[[HANDLER_IMPLICIT_CAST:.*]], !prof ![[WEIGHT_MD:.*]], !nosanitize + // CHECK-SANITIZE-TRAP-NEXT: br i1 %[[TRUNCHECK]], label %[[CONT:.*]], label %[[HANDLER_IMPLICIT_CAST:.*]], !nosanitize ---------------- vsk wrote: > There's no need to check the profile metadata here. I was checking it because otherwise `HANDLER_IMPLICIT_CAST` would have over-eagerly consumed `, !prof !3` too. But there is actually a way around that.. ================ Comment at: test/CodeGen/catch-implicit-integer-truncations.c:159 +// ========================================================================== // +// The expected false-negatives. +// ========================================================================== // ---------------- vsk wrote: > nit, aren't these true-negatives because we expect to see no errors? Right. Repository: rC Clang https://reviews.llvm.org/D48958 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
