aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from the testing requests.
================ Comment at: clang/lib/Basic/IdentifierTable.cpp:718-719 + case NullabilityKind::NullableResult: + assert(!isContextSensitive && + "_Nullable_result isn't supported as context-sensitive keyword"); + return "_Nullable_result"; ---------------- erik.pilkington wrote: > aaron.ballman wrote: > > Can you explain why it differs from `_Nullable` in this case? > 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. Ah, thank you for the explanation! ================ Comment at: clang/test/SemaObjC/nullability.m:119 - (null_unspecified id)returnsNullUnspecified; +- (_Nullable_result id)returnsNullableResult; @end ---------------- Can you add a test case that shows `- (nullable_result id)returnsNullableResultErr;` doesn't work (but also doesn't do something bad, like assert)? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92495/new/ https://reviews.llvm.org/D92495 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
