RFR: 8253089: Windows (MSVC 2017) build fails after JDK-8243208

[Kim Barrett]

> On Sep 14, 2020, at 9:05 AM, Aleksey Shipilev <shade at openjdk.java.net> wrote:
> 
> It seems that MSVC 2017 is getting confused about the differences in `unsigned int` and `u2`. After a few attempts at
> fixing this, I think we need to use `u2` consistently for hash code computations.
> 
> -------------
> 
> Commit messages:
> - No need to cast to short
> - Indenting
> - Minor cleanup
> - Another try, use u2 consistently
> - 8253089: Windows (MSVC 2017) build fails after JDK-8243208
> 
> Changes: https://git.openjdk.java.net/jdk/pull/150/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=150&range=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8253089
>  Stats: 8 lines in 2 files changed: 0 ins; 0 del; 8 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/150.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/150/head:pull/150
> 
> PR: https://git.openjdk.java.net/jdk/pull/150

I suspect the compiler warning is an example of
https://developercommunity.visualstudio.com/content/problem/211134/unsigned-integer-overflows-in-constexpr-functionsa.html

This would explain why using u2 would dodge the warning. The currently
overflowing arithmetic operation would be on the promoted u2 values, so
won't overflow.

I think changing to using u2 isn't the right solution though.  This throws
away a lot of bits in the hash-code calculation, potentially making it less
effective.  I think a better workaround is to locally suppress the warning,
which I think is from the multiplication here:

61       h = 31*h + (unsigned int) *s; 

So I suggest wrapping that line with targeted warning suppression, i.e.
using PRAGMA_DIAG_PUSH/POP and PRAGMA_DISABLE_MSVC_WARNING(4307) with an
appropriate comment. I don't love cluttering code with that kind of thing,
but I think in this case that's a better solution than changing what's
being calculated.

Note that it might be that such a limited warning disable is insufficient in
the long run. This seems like a problem that might arise in other places as
we make more use of constexpr. We might instead need to globally disable
that warning for some limited range of VS versions.