Add nullability marker annotations #344

TheMrMilchmann · 2017-11-05T22:50:56Z

As previously discussed in Slack, this PR adds JSR-305 like annotations to support null-analysis and to enhance Kotlin interopertability. (See here for more info.)

@Spasi As suggested I did not add @Nullable to most stack allocation methods (the exception being String conversion methods, since they can also be null when the passed CharSequence is null).
I'm open for further suggestions and will implement them ASAP if requested.

TheMrMilchmann · 2017-12-03T14:37:00Z

I just cloned this branch into a fresh environment and realized that the latest commits seem to be missing some changes. I'll quickly rebase and fix this.

TheMrMilchmann · 2017-12-05T14:14:33Z

Rebased and updated. Should be good to go now (this time for real)!

TheMrMilchmann

For the most part, this is looking good. All I found was one issue and a few nitpicky parts.

I would like to hear your opinions first before I change anything.

TheMrMilchmann · 2018-01-10T09:33:11Z

modules/generator/src/main/kotlin/org/lwjgl/generator/Types.kt

@@ -365,6 +367,9 @@ val NativeType.isPointer
 val NativeType.isPointerData
    get() = this is PointerType && this.mapping !== PointerMapping.OPAQUE_POINTER

+val NativeType.isPointerObject
+    get() = this is PointerType && (this.mapping !== PointerMapping.OPAQUE_POINTER || this is ObjectType)


This, currently, does not seem to include JObjectTypes which I think I fixed and is something that was lost at some point. IIRC this was also the reason to make JObjectType a class.

Changing the expression to (this is PointerType && (this.mapping !== PointerMapping.OPAQUE_POINTER || this is ObjectType)) || this is JObjectType should to the trick.

Thanks! I refactored some aspects of the type system a bit and this was simplified to:

this !is ValueType && (this.mapping !== PointerMapping.OPAQUE_POINTER || this is ObjectType)

The property was also renamed to isReference, which I think is more appropriate.

TheMrMilchmann · 2018-01-10T09:38:59Z

modules/core/src/main/java/org/lwjgl/system/MemoryStack.java

@@ -531,23 +532,31 @@ public ByteBuffer ASCII(CharSequence text) {
    /**
     * Encodes the specified text on the stack using ASCII encoding and returns a ByteBuffer that points to the encoded text.
     *
-     * @param text           the text to encode. If {@code text} is null, null is returned.
+     * @param text           the text to encode
     * @param nullTerminated if true, a null-terminator is included at the end of the encoded text
     */
    public ByteBuffer ASCII(CharSequence text, boolean nullTerminated) {


Since the documentation of "text encoding"-methods does not refer to any "allocation"-method we should probably explicitly document what happens if the operation fails here.

There's nothing special about the encoding methods, they fail in the same way as other stack allocation methods with an OutOfMemoryError("Out of stack space.").

TheMrMilchmann · 2018-01-10T09:41:26Z

modules/core/src/main/java/org/lwjgl/system/MemoryUtil.java

@@ -442,6 +493,19 @@ public static long nmemAlignedAlloc(long alignment, long size) {
        return ALLOCATOR.aligned_alloc(alignment, size);
    }

+    public static long nmemAlignedAllocChecked(long alignment, long size) {


This method is missing documentation.

TheMrMilchmann · 2018-01-10T09:53:38Z

doc/notes/3.1.6.md

+- Added additional nullability information with [JSR-305](https://jcp.org/en/jsr/detail?id=305) annotations.
+    * Enables null-analysis to be used to improve the code quality of LWJGL applications.
+    * Improves type-safety for applications using LWJGL in Kotlin
+    ([Read the Kotlin documentation](https://kotlinlang.org/docs/reference/java-interop.html#jsr-305-support) for more info.) 


There should probably be a dot (.) at the end of the last bullet point.

I'm not quite sure whether I like the current wording. Any objections?

I have changed the wording a bit.

Spasi · 2018-01-11T22:53:40Z

The next snapshot (3.1.6 build 9) will have all the latest updates in bindings, but not the nullability annotations. It will be promoted to stable when it's up. Starting from build 10, the above changes will be available for testing.

Thank you @TheMrMilchmann for the great work!

Spasi added Status: In Progress Type: Enhancement labels Nov 7, 2017

TheMrMilchmann force-pushed the nullability-markers branch from 7246c08 to 1df8a28 Compare November 8, 2017 13:10

Spasi force-pushed the nullability-markers branch from 1df8a28 to 0b4e712 Compare November 9, 2017 16:35

Spasi force-pushed the nullability-markers branch from 3958ee4 to 039d78f Compare November 23, 2017 22:11

TheMrMilchmann force-pushed the nullability-markers branch 2 times, most recently from 9e237fe to c49cffa Compare December 5, 2017 13:52

Spasi force-pushed the nullability-markers branch from 2b947c2 to e1a7372 Compare January 10, 2018 08:51

TheMrMilchmann commented Jan 10, 2018

View reviewed changes

Spasi force-pushed the nullability-markers branch from e1a7372 to 17976eb Compare January 10, 2018 17:55

TheMrMilchmann and others added 4 commits January 12, 2018 00:34

feat: add nullability annotations

9cfbe25

feat: add Nonnull modifier for return values

8e56d27

feat: nullability support in structs, callbacks, memory management

c53ffeb

feat: use nullability information in tests/demos

82f02e5

Spasi force-pushed the nullability-markers branch from 17976eb to 82f02e5 Compare January 11, 2018 22:44

Spasi merged commit 737d011 into LWJGL:master Jan 11, 2018

TheMrMilchmann deleted the nullability-markers branch January 12, 2018 13:17

Spasi added Status: Completed and removed Status: In Progress labels Jan 12, 2018

Spasi mentioned this pull request Jan 29, 2018

Eclipse: The type ... cannot be resolved. It is indirectly referenced from required .class files #369

Closed

TheMrMilchmann mentioned this pull request Oct 11, 2024

Migrate to JSpecify nullability annotations #1008

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add nullability marker annotations #344

Add nullability marker annotations #344

TheMrMilchmann commented Nov 5, 2017

TheMrMilchmann commented Dec 3, 2017

TheMrMilchmann commented Dec 5, 2017

TheMrMilchmann left a comment

TheMrMilchmann Jan 10, 2018

Spasi Jan 10, 2018

TheMrMilchmann Jan 10, 2018

Spasi Jan 10, 2018

TheMrMilchmann Jan 10, 2018

Spasi Jan 10, 2018

TheMrMilchmann Jan 10, 2018

Spasi Jan 10, 2018

Spasi commented Jan 11, 2018

Add nullability marker annotations #344

Add nullability marker annotations #344

Conversation

TheMrMilchmann commented Nov 5, 2017

TheMrMilchmann commented Dec 3, 2017

TheMrMilchmann commented Dec 5, 2017

TheMrMilchmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Spasi commented Jan 11, 2018