On Wed, 20 Jul 2022 14:18:49 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> This patch enables lossy conversion warnings (C4244 [1]) for hotspot on >> Windows/MSVC. Instead of fixing all warnings that were produced from this, >> I've instead locally disabled the warning in the files that produced >> warnings. This allows gradually making progress with cleaning up these >> warnings on a per-file basis, instead of trying to fix all of them in one >> shot. i.e. it is not meant as a long term solution, but as a way of allowing >> incremental progress. >> >> Out of the ~1100 files that make up hotspot on Windows x64 , ~290 have >> warnings for them disabled (not counting aarch64 files), which means that >> with this patch ~800 files are protected by enabling this warning globally. >> >> Warnings can be fixed in individual files, or groups of files in followup >> patches, and warnings for those files can be enabled. >> >> I'm working on a patch that does the same for GCC, but it produces warnings >> in about 150 more files, so I wanted to gather feedback on this approach >> before continuing with that. >> >> --- >> >> To disable warnings for a file, in most cases the following prelude is added >> after the last `#include` at the start of a file: >> >> PRAGMA_DIAG_PUSH >> PRAGMA_ALLOW_LOSSY_CONVERSIONS >> >> And then the following is added at the end of the file for cpp files, or >> before closing the header guard for hpp files: >> >> PRAGMA_DIAG_POP >> >> 1 notable exception are files produced by adlc, which had their code-gen >> modified to add these lines instead. There were also 2 files that include >> headers in the middle of the file (ostream.cpp & sharedRuntime.cpp), for >> which I've added the PRAGMA's after the include block at the start of the >> file instead. They only included system headers, for which disabling >> warnings doesn't matter any ways. >> >> [1]: >> https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-levels-3-and-4-c4244?view=msvc-170 > > 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 I don't think this will go anywhere, so I'll just close the PR. Thanks for the discussion on the idea! ------------- PR: https://git.openjdk.org/jdk/pull/9516
