On Tue, 8 Aug 2023 19:56:24 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:

>> 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/9da8c164...51230f3d
>
> 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 the compiler complaining? If so, 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).

Hi Thomas, the goto jumps over a lot of variable initializations, which 
permissive- doesn't particularly like (the failing compilation can actually be 
seen in the earlier GHA logs)

> 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?

This is a weird one, but in awt we #define malloc Do_Not_Use_Malloc... And so 
on. Without this reordering awt_ole.h (which includes comdef.h) also uses the 
redefined malloc somewhere (I could not find where in comip.h that it's used) 
which breaks compilation. I do find it strange that without permissive- it 
doesn't break though. Same applies to the other reordering of includes

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1287904601
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1287903377

Reply via email to