New nullable annotation

libgit2 / objective-git

Objective-C bindings to libgit2

MIT License

1.16k stars 280 forks source link

New nullable annotation #582

Closed alehed closed 7 years ago

alehed commented 8 years ago

While I'm at it. Also removes some nullability annotations from the implementation files. Depends on #580 to be merged first.

tiennou commented 8 years ago

Any specific reason for the change (other that it's new in Xcode 7) ? I find that painful-er to read, and I haven't been able to find what's the current preferred syntax (Apple seems to use the non-underscored variant unless there's wacky pointers-to-pointers).

This and this seem to imply that _Nullable was added for compatibility with C code that already used __nullable, and that nullable is only available to Obj-C.

Just my 2 cents, in any case.

alehed commented 8 years ago

The main argument is consistency. It avoids the use of both __nullable, nullable and possibly _Nullable in the same file. Also it says in the blog post that in Xcode 7 __nullable is now just a macro that expands to _Nullable.

As to the aesthetic consideration, I agree the property syntax with nullable looks nicer but in the other cases I think the older syntax is not justified and also this is Obj-C we're talking about.

Anyway, @phatblat suggested it so I thought it would be a good time to update the syntax.

alehed commented 8 years ago

If you think it is cleaner, I can revert it in the @property cases. In the other cases I think _Nullable is the way to go.

pietbrauer commented 7 years ago

@alehed Thanks for the PullReuqest. I also don't like it but it seems this is the way to go in Xcode 7 and above. Could you rebase onto master so we can merge this?

alehed commented 7 years ago

Should I leave the ones with @property syntax alone or should everything be _Nullable?

pietbrauer commented 7 years ago

I would go with the new Xcode way and use _Nullable

tiennou commented 7 years ago

Those _nullable annotations are for compatibility with someones' else C attribute that was found to be clashing.

From the Nullability and Objective-C document linked above, emphasis mine :

You can use _Nullable and _Nonnull almost anywhere you can use the normal C const keyword, though of course they have to apply to a pointer type. However, in the common case there’s a much nicer way to write these annotations: within method declarations you can use the non-underscored forms nullable and nonnull immediately after an open parenthesis, as long as the type is a simple object or block pointer.
This feature was first released in Xcode 6.3 with the keywords nullable and nonnull. Due to potential conflicts with third-party libraries, we’ve changed them in Xcode 7 to the _Nullable and _Nonnull you see here.

alehed commented 7 years ago

Rebased on top of master.

pietbrauer commented 7 years ago

Thanks a lot for your contribution!