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

Jorn Vernee jvernee at openjdk.org
Wed Jul 20 14:46:04 UTC 2022


On Wed, 20 Jul 2022 14:18:45 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Jorn Vernee has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Polish
>>  - Remove PUSH POP from test files
>
> This feels to me like rather a blunt instrument. IMO, it would be better to use checked_cast() where lossy conversions happen. That would make code easier to understand, and it'd give us warnings when things go wrong.

@theRealAph I think ultimately, each warning site should be inspected, and a bespoke solution should be found. Inserting `checked_cast` is only one of a possible set of solutions. Others being changing the type of variables used, or some wider refactoring.

This change was mostly applied mechanically by parsing the build log, and then using a script to insert these lines, with a manual review + 1 or 2 fixups. Making sure the inserted `checked_cast`s are correct seems much harder.

I think having `PRAGMA_ALLOW_LOSSY_CONVERSIONS` in the file also sends a much clearer message that: warnings in this file have not been looked at/fixed yet, which I don't think `checked_cast` does.

I have my doubts that adding `checked_cast` everywhere is always correct. In some cases the truncation might be the desired behaviour (just not made explicit with a cast), and I don't want to run the risk of breaking such code where the tests don't catch it (while I also don't want to get into a lengthy process of fixing up those cases one by one).

The approach I've taken preserves the current behavior of the code, but it also allows for fixing these warnings on a per-file basis. This seems to me like an easier and safer way to make progress.

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

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



More information about the build-dev mailing list