[PATCH] D23236: When ARC is enabled, no warning will be generated when a method1. Returns 'nil' in a method that is attributed to return a 'nonnull'2. The return-statement is a ConditionalOperator, where the lhs is nil and rhs an objC-method-call (or the other...

Hendrik von Prince via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 23 08:11:04 PDT 2016


parallaxe marked 4 inline comments as done.
parallaxe added a comment.

In https://reviews.llvm.org/D23236#509013, @ahatanak wrote:

> +cfe-commits
>
> If this patch is applied, does clang issue a warning if a method marked "nonnull" returns a null value? I see a warning is issued for conditional expressions in the test case you've added, but I don't see a test case for a function returning just nil or 0.
>
> I was wondering whether the change made in this patch contradicts what's stated in r240146's commit log:
>
> "Note that we don't warn about nil returns from Objective-C methods,
>
>   because it's common for Objective-C methods to mimic the nil-swallowing
>   behavior of the receiver by checking ostensibly non-null parameters
>   and returning nil from otherwise non-null methods in that
>   case."


I'm not sure how this is meant. As far as I understand the commit-message you quoted, it is related to methods that return nil when a precondition fails. A test for this would be `doNotWarnWhenPreconditionIsViolatedInTopFunc` in nullability_nullonly.mm, which is not affected by my patch. 
On the other side, according to the test `testReturnsNilInNonnull` in nullability_nullonly.mm

  - (SomeClass * _Nonnull)testReturnsNilInNonnull {
    SomeClass *local = nil;
    return local; // expected-warning {{Null is returned from a method that is expected to return a non-null value}}
  }

it is expected to produce a warning, when a method marked as nonnull returns nil while no precondition is violated. 
So I would expect that the existing tests already proof that my patch didn't produce unwanted side-effects.

Sorry in case that I'm missing your point here, maybe you can elaborate it a bit more?


https://reviews.llvm.org/D23236





More information about the cfe-commits mailing list