RFR(s): 8221408: Windows 32bit build build errors/warnings in hotspot

[Kim Barrett]

> On Mar 27, 2019, at 1:41 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> 
> Hi Kim,
> 
> thanks for looking a this. Pls see my remarks inline.
> 
> On Wed, Mar 27, 2019 at 5:59 AM Kim Barrett <kim.barrett at oracle.com> wrote:
> > The warning suggests an implicit narrowing in an initializer list,
> > which is forbidden except under certain circumstances where the source
> > is a constant expression whose value will fit in the target type; see
> > C++14 8.5.4/7.
> > 
> > But I don't see why that warning is being triggered here.
> > hash_mask_in_place is (for Win32) an enum value,
> 
> Actually, it’s an enum for everything *except* Win64.  I got confused by the
> conditionalization, but I think the discussion below is still mostly okay.
> 
> 
> Yes, that is why I shied away from such a change and rather fixed the assignment itself.
> 
> That and the feeling that the code was perfectly fine as it is; changing it by replacing the enum value with an uintptr_t felt like cargo-cult programming. To satisfy one broken compiler.

I suspect we wouldn't even be discussing this if we were using static consts
with known types rather than enum values with implicit platform-dependent
types. And the driver for much of our enum usage is ancient broken compilers
that haven't been usable by us for *many* years (JDK-8153116).

Anyway, a cleaned up version of my patch passed tier1 on Oracle-supported platforms.

> The more I think about this the more I think I would rather manually switch the warning off on WIN32 with a pragma. Difficult to do this though for just one value in the macro list. However, that would be the clearest expression of "we have a broken compiler on one platform but the code is actually fine”.

My preference is to make the change to static consts.  Indeed, as a cleanup I’d get replace
that enum (and probably those nearby) with static consts with explicit types, rather than the
platform-dependent implicit enum types.

Before going any further though, I’d want to know whether my patch actually does fix the
win32 build failure, or if there is something else lurking there.