RFR: 8307160: [REDO] Enable the permissive- flag on the Microsoft Visual C compiler [v2]

[Thomas Stuefe]

On Mon, 7 Aug 2023 06:42:41 GMT, Julian Waters <jwaters at openjdk.org> wrote:

>> We should set the -permissive- flag for the Microsoft Visual C compiler, as was requested by the now backed out [JDK-8241499](https://bugs.openjdk.org/browse/JDK-8241499). It can be done with some effort, given that the significantly stricter gcc can now compile an experimental Windows JDK as of 2023, and will serve to significantly cut down on monstrosities in ancient Windows code
>
> Julian Waters has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 22 additional commits since the last revision:
> 
>  - Mismatched declaration in D3DGlyphCache.cpp
>  - Fields in awt_TextComponent.cpp
>  - reinterpret_cast needed in AccessBridgeJavaEntryPoints.cpp
>  - Qualifiers in awt_PrintDialog.h should be removed
>  - Likewise for awt_DnDDT.cpp
>  - awt_ole.h include order issue in awt_DnDDS.cpp
>  - Revert awt_ole.h
>  - Earlier fix in awt_ole.h was not complete
>  - Merge branch 'openjdk:master' into patch-10
>  - Likewise for awt_Frame.cpp
>  - ... and 12 more: https://git.openjdk.org/jdk/compare/c9dcbf20...51230f3d

Hi Julian,

Cursory review. I stopped halfway in when I noticed that I had no idea why many of the changes were done. They did not seem to have an obvious connection to the title of the JBS, and I did not find an explanation.

I also find the usage of "monstrosities" unnecessary. It is very much a matter of taste - to me, nothing in the code you changed looks monstrous.

Cheers, Thomas

src/hotspot/os/windows/os_windows.cpp line 2888:

> 2886: LONG WINAPI topLevelUnhandledExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo) {
> 2887:   if (!InterceptOSException) {
> 2888:     DWORD exception_code = exceptionInfo->ExceptionRecord->ExceptionCode;

I find this less clearer than the original code, that didn't use negation - it was clear that InterceptOSException leads to immediate bailout.

What is the problem, the goto? Is that just your personal taste, or is the compiler complaining? If the latter, I would vote for duplicating the return statement here (and this would count as an example where discarding out-of-fashion statements like goto actually makes the code worse).

src/java.desktop/windows/native/libawt/windows/awt_Canvas.cpp line 216:

> 214:     {
> 215:         PDATA pData;
> 216:         JNI_CHECK_PEER_GOTO(canvas, ret);

Here, and other places: why this scope?

src/java.desktop/windows/native/libawt/windows/awt_DnDDS.cpp line 48:

> 46: 
> 47: #include "jlong.h"
> 48: #include "awt.h"

Why the reordering of includes?

src/java.desktop/windows/native/libawt/windows/awt_DnDDT.cpp line 34:

> 32: #include "sun_awt_windows_WDropTargetContextPeer.h"
> 33: #include "awt_Container.h"
> 34: #include "awt_ole.h"

Why?

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

PR Review: https://git.openjdk.org/jdk/pull/15096#pullrequestreview-1568088318
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1287629714
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1287630358
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1287631591
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1287632623