Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New nullable annotation #582

Merged
merged 3 commits into from
Mar 10, 2017
Merged

Conversation

alehed
Copy link
Contributor

@alehed alehed commented Jun 28, 2016

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

@tiennou
Copy link
Contributor

tiennou commented Jun 28, 2016

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
Copy link
Contributor Author

alehed commented Jun 28, 2016

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
Copy link
Contributor Author

alehed commented Nov 5, 2016

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
Copy link
Member

@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
Copy link
Contributor Author

alehed commented Feb 27, 2017

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

@pietbrauer
Copy link
Member

I would go with the new Xcode way and use _Nullable

@tiennou
Copy link
Contributor

tiennou commented Feb 27, 2017

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 alehed force-pushed the new_nullable_annotation branch from ceca0f5 to 9e4468b Compare March 9, 2017 17:27
@alehed alehed force-pushed the new_nullable_annotation branch from 4e5f9e9 to 7de377e Compare March 9, 2017 17:50
@alehed
Copy link
Contributor Author

alehed commented Mar 9, 2017

Rebased on top of master.

@pietbrauer pietbrauer merged commit ff3bd77 into libgit2:master Mar 10, 2017
@pietbrauer
Copy link
Member

Thanks a lot for your contribution!

@alehed alehed deleted the new_nullable_annotation branch March 10, 2017 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants