On Fri, 15 Jul 2022 13:25:57 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.
> 
> 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

I thought PRAGMA_DIAG_PUSH/POP are for narrowing the scope of the pragma.  So 
unless we are concatenating .cpp files together, I would think they are not 
needed for .cpp files if we want to affect the whole file.

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

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

Reply via email to