ahatanak added a comment. In https://reviews.llvm.org/D23236#523173, @parallaxe wrote:
> 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? Thank you for the explanation, I think I understand the comment now. I was wondering whether the analyzer was being overzealous since r240153's commit log clearly states that Sema never warns about nonnull methods returning nil, but I agree with you that probably it was just trying to avoid false positive warnings for functions like doNotWarnWhenPreconditionIsViolatedInTopFunc. https://reviews.llvm.org/D23236 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
