[llvm] r224195 - Silencing a -Wsign-compare warning; NFC.

[Aaron Ballman]

On Sun, Dec 14, 2014 at 5:32 PM, Chandler Carruth <chandlerc at google.com> wrote:
>
> On Sat, Dec 13, 2014 at 8:55 AM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
>>
>>        uint64_t Shift = 64 - EmissionSize * 8;
>> -      assert(Shift < std::numeric_limits<unsigned long long>::digits &&
>> +      assert(Shift < static_cast<unsigned>(
>> +                         std::numeric_limits<unsigned long long>::digits)
>> &&
>
>
> The LHS is a 64-bit number here, it seems odd to cast to a (potentially)
> 32-bit number.

I've rectified that in r224249 -- that was a mistake on my part.

> Why couldn't all of these be signed integers, and we get UBSan failures if
> the math is wrong? I really dislike this use of unsigned here, and I feel
> that numeric_limits got this right -- these are small "numbers" and "int"
> seems like a really nice type for them. =]

(I'm not the author of this code, originally.)

UBSan doesn't work on every platform, which may be a reason. That
being said, I don't understand why this code requires a 64-bit integer
in the first place. All of the calculations involved to this point
seem like they would fit within an unsigned without issue. As for
"int" vs "unsigned", I think unsigned is more appropriate if these
values are never expected to be negative in the first place, but don't
have incredibly strong opinions on it.

~Aaron