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