This is an archive of the discontinued LLVM Phabricator instance.

[clang] Add a new nullability annotation for swift async: _Nullable_result
ClosedPublic

Authored by erik.pilkington on Dec 2 2020, 10:07 AM.

Diff Detail

Event Timeline

erik.pilkington requested review of this revision.Dec 2 2020, 10:07 AM
doug.gregor accepted this revision.Dec 2 2020, 12:10 PM

This looks great to me, thank you!

This revision is now accepted and ready to land.Dec 2 2020, 12:10 PM
aaron.ballman added inline comments.Dec 3 2020, 9:34 AM
clang/include/clang/Basic/AttrDocs.td
3516

its -> it's

3526

Swift imported -> Swift importer

clang/lib/Basic/IdentifierTable.cpp
718–719

Can you explain why it differs from _Nullable in this case?

clang/lib/Sema/SemaExprObjC.cpp
1566

Should that be auto *?

1573

Same here.

erik.pilkington marked 5 inline comments as done.

Address review comments.

clang/lib/Basic/IdentifierTable.cpp
718–719

Sure, _Nullable can be written like nullable in an @property or method return/param type i.e.:

@property(nullable) id x;

_Nullable_result isn't really useful in these contexts (you should always use nullable there), so I didn't add parsing support for it.

clang/lib/Sema/SemaExprObjC.cpp
1566

Its an Optional<NullabilityKind>, so I guess it should really be written out. New patch drops the auto.

doug.gregor added inline comments.Dec 4 2020, 9:24 AM
clang/test/SemaObjC/nullable-result.m
7

Can you add a __has_attribute check for _Nullable_result?

aaron.ballman accepted this revision.Dec 4 2020, 9:36 AM

LGTM aside from the testing requests.

clang/lib/Basic/IdentifierTable.cpp
718–719

Ah, thank you for the explanation!

clang/test/SemaObjC/nullability.m
119

Can you add a test case that shows - (nullable_result id)returnsNullableResultErr; doesn't work (but also doesn't do something bad, like assert)?

erik.pilkington marked 3 inline comments as done.

Address review comments.

clang/test/SemaObjC/nullable-result.m
7

Hmm, it looks like __has_attribute doesn't support keyword attributes. I'll add a __has_feature.

Herald added a project: Restricted Project. · View Herald TranscriptDec 7 2020, 2:20 PM