[PATCH] D48958: [clang][ubsan] Implicit Cast Sanitizer - integer truncation - clang part

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D48958#1173860, @vsk wrote:

> LGTM, although I think it'd be helpful to have another +1 just to be safe.


Thank you for the review!
It would indeed be great if someone else could take a look, especially since we are **so** close to the branching point.

In https://reviews.llvm.org/D48958#1173860, @vsk wrote:

> ...




In https://reviews.llvm.org/D48958#1173860, @vsk wrote:

> These four at least don't look like false positives:
>
> - Maybe we should consider special-casing assignments of "-1" to unsigned values? This seems somewhat idiomatic.


I **personally** would use `~0U` there.
One more datapoint: the `implicit-sign-change` will/should still complain about that case.
So **personally** i'd like to keep it.

> - At least a few of these are due to not being explicit about dropping the high bits of hash_combine()'s result. Given that this check is opt-in, that that seems like a legitimate diagnostic (lost entropy).
> - The TargetLoweringBase.cpp diagnostic looks a bit scary.




Repository:
  rC Clang

https://reviews.llvm.org/D48958