On Thu, 28 Mar 2024 07:36:04 GMT, Julian Waters <jwat...@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). Doing so makes 
>> the Visual C compiler much less accepting of ill formed code, which will 
>> improve code quality on Windows in the future.
>
> Julian Waters has updated the pull request incrementally with four additional 
> commits since the last revision:
> 
>  - Labels to empty line in awt_Window.cpp
>  - Labels to empty line in awt_Window.cpp
>  - Label to empty line in awt_Window.cpp
>  - Label to empty line in awt_Window.cpp

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

> 232:     c->m_eraseBackgroundOnResize = doEraseOnResize;
> 233: 
> 234: 

I recommend deleting this line, since it does not make sense to have two 
consecutive blank lines here.

src/java.desktop/windows/native/libawt/windows/awt_Component.cpp line 6586:

> 6584:     component->GetInsets(gis->insets);
> 6585: 
> 6586: 

I recommend deleting this line, since it does not make sense to have two 
consecutive blank lines here.

src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp line 1603:

> 1601:     } else {
> 1602:         f = (AwtFrame *) JNI_GET_PDATA(peer);
> 1603:         if (f == nullptr) {

@prrace What is the current take on `NULL` vs `nullptr` in client code? I know 
Hotspot made an effort to completely purge all `NULL` references. 

The new code here uses a mix of `NULL` and `nullprt`. Should it:
a) only use `NULL`
b) only use `nullptr`
c) remain as it is
?

src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 214:

> 212: static const double POINTS_TO_LOMETRIC = (254.0 / 72.0);
> 213: 
> 214: 

I recommend deleting one more line, since otherwise you will now have three 
consecutive blank lines.

src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 1031:

> 1029:     window->RepositionSecurityWarning(env);
> 1030: 
> 1031:   ret:

I recommend deleting this line, since it does not make sense to have two 
consecutive blank lines here.

src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3159:

> 3157:     }
> 3158: 
> 3159: 

I recommend deleting this line, since it does not make sense to have two 
consecutive blank lines here.

src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3193:

> 3191:     }
> 3192: 
> 3193: 

I recommend deleting this line, since it does not make sense to have two 
consecutive blank lines here.

src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3224:

> 3222:     window->SetTranslucency(iOpacity, window->isOpaque());
> 3223: 
> 3224: 

I recommend deleting this line, since it does not make sense to have two 
consecutive blank lines here.

src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3255:

> 3253:     window->SetTranslucency(window->getOpacity(), isOpaque);
> 3254: 
> 3255: 

I recommend deleting this line, since it does not make sense to have two 
consecutive blank lines here.

src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3293:

> 3291:                          uws->hBitmap);
> 3292: 
> 3293: 

I recommend deleting this line, since it does not make sense to have two 
consecutive blank lines here.

src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 3328:

> 3326:     window->setFullScreenExclusiveModeState(state != 0);
> 3327: 
> 3328: 

I recommend deleting this line, since it does not make sense to have two 
consecutive blank lines here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547824279
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547824797
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547829415
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547830886
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547833185
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547836998
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547837189
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547837344
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547837450
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547837625
PR Review Comment: https://git.openjdk.org/jdk/pull/15096#discussion_r1547837761

Reply via email to