On Wed, 20 Jul 2022 14:18:45 GMT, Andrew Haley <[email protected]> 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
