On Thu, 22 Jun 2023 14:40:23 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> On Windows, the basic Java Integer types are defined as long and __int64 >> respectively. In particular, the former is rather problematic since it >> breaks compilation as the Visual C++ becomes stricter and more compliant >> with every release, which means the way Windows code treats long as a >> typedef for int is no longer correct, especially with -permissive- enabled. >> Instead of changing every piece of broken code to match the jint = long >> typedef, which is far too time consuming, we can instead change jint to an >> int (which is still the same 32 bit number type as long), as there are far >> fewer problems caused by this definition. It's better to get this over and >> done with sooner than later when a future version of Visual C++ finally >> starts to break on existing code > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Revert "GetDIBits should take an LPVOID" > > This reverts commit 7dbe5dea84b1afb2235b66da581bcd3c1da4d6ac. Looks fine to me, except for a few comments. The size of the types for `jint` and `jlong` remains the same after amending the typedef `jni_md.h`. Yet I'm still cautious about it. You should have an approval from hotspot. Please also update the copyright year in the modified files. src/java.desktop/windows/native/libawt/java2d/windows/GDIRenderer.cpp line 325: > 323: } > 324: > 325: jint sx, sy, ex, ey; I agree with David, these should have type `int` as accepted by the [`::Arc`](https://learn.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-arc) function. However, the `AngleToCoord` function is declared with `jint` as parameters. https://github.com/openjdk/jdk/blob/84f8e08c2ecc90ec50a13406fb99b8cd52f33b7c/src/java.desktop/windows/native/libawt/java2d/windows/GDIRenderer.cpp#L52 Its declaration can be changed to `int`, it's an internal function used by `*_doDrawArc` and `*_doFillArc`. src/java.desktop/windows/native/libawt/windows/ShellFolder2.cpp line 1084: > 1082: > 1083: jint *colorBits = nullptr; > 1084: int *maskBits = nullptr; Suggestion: jint *colorBits = NULL; int *maskBits = NULL; I'd rather keep `NULL` — it's used consistently inside `_Win32ShellFolder2_getIconBits` function as well as through the file, so `nullptr` is out of place. src/java.desktop/windows/native/libawt/windows/awt_MenuBar.cpp line 148: > 146: } > 147: > 148: AwtMenuItem* AwtMenuBar::GetItem(jobject target, jint index) What is the reason for using `jint` instead of `int`? The member function is used in for-loop which iterates with `int` loop variable. Yet the implementation of `GetItem` up-calls into Java. ------------- PR Review: https://git.openjdk.org/jdk/pull/14125#pullrequestreview-1493854643 PR Review Comment: https://git.openjdk.org/jdk/pull/14125#discussion_r1238938194 PR Review Comment: https://git.openjdk.org/jdk/pull/14125#discussion_r1238948758 PR Review Comment: https://git.openjdk.org/jdk/pull/14125#discussion_r1238966069
