RFR: 8290373: Enable lossy conversion warnings on Windows [v4]

[Jorn Vernee]

On Mon, 8 Aug 2022 12:47:52 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> Jorn Vernee has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 15 commits:
>> 
>>  - Merge branch 'master' into Warn_Narrow
>>  - Polish pt2
>>  - Polish
>>  - Remove PUSH POP from test files
>>  - Remove PUSH POP from cpp files
>>  - Rest of the tests
>>  - More test
>>  - AArch64
>>  - Disable for tests
>>  - Fix apostrophe
>>  - ... and 5 more: https://git.openjdk.org/jdk/compare/1c055076...fb276afd
>
> While I normally appreciate enabling warnings, and fixing them, I tend to side a bit with David on this one. There is a lot of `PRAGMA_ALLOW_LOSSY_CONVERSIONS` in this PR, injected into the files, forever to be forgotten.
> 
> I do agree with @JornVernee  that warnings are in a bit of a bind; if we really think a warning is useful, but it has been historically turned off, it is a huge uphill battle to get it enabled, by fixing all historical issues at once. This results in warnings being left turned off, even for new code, since no-one has the time available to address all issues at once.
> 
> I'm thinking that part of this problem is due to the lack of granularity that the build systems offers to enable/disable warnings. We have a "global" enable, and then we have a "per dll" disable.
> 
> It would not be too hard for me to modify this, so we could make a list of source files for which the conversion warning still needs to be turned off. If this is a coherent list in a single make file, it is much easier to see what remains to be done to actually fix these issues. (This will not work for .hpp files; these will still need the pragma push/pop thing, and should therefore be prioritized when actually fixing this.)
> 
> Still, the warnings need to be fixed (or the list will need to be kept indefinitely, which is of course also a valid solution). And we need to be prepared to put in effort to actually resolve these, if we really want this to happen.

Thanks @magicus turning off warnings in a side file also seems like a decent solution.

I think part of the problem with tackling all warnings at once is that it's (too) hard to decide whether it's worth doing, since there's such a huge volume of warnings. It's much easier to decide that for a couple of files, and then put in the work. i.e. employ a divide and conquer strategy.

Maybe some of the warnings will never be fixed, because it just isn't worth the effort in some cases. But, this still seems better than globally disabling the warning to me.

I think it's mostly an organizational/workflow issue, rather than a technical one.

-------------

PR: https://git.openjdk.org/jdk/pull/9516