This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Refine the diagnostics in the nullability checker to differentiate between nil and null
ClosedPublic

Authored by zaks.anna on Dec 8 2016, 4:11 PM.

Details

Summary

This is a big deal for ObjC, where nullability annotations are extensively used. I've also changed "Null" -> "null" and removed "is" as this is the pattern that Sema is using.

Diff Detail

Repository
rL LLVM

Event Timeline

zaks.anna updated this revision to Diff 80843.Dec 8 2016, 4:11 PM
zaks.anna retitled this revision from to [analyzer] Refine the diagnostics in the nullability checker to differentiate between nil and null.
zaks.anna updated this object.
zaks.anna added a reviewer: dcoughlin.
zaks.anna added subscribers: dergachev.a, cfe-commits.
zaks.anna added a subscriber: xazax.hun.
dcoughlin edited edge metadata.Dec 9 2016, 9:05 AM

I think in the 'null' case it might be better to keep it as "Null passed" or even "Null value passed". This is different than the 'nil' case because the diagnostic is not referring to a literal.

Looks like Sema uses "null" not only when referring to literals (see below). Also, if we were referring to literals, we would use single quotes, no?

I suggest keeping as is for consistency with the wording that uses nil. I do not see much difference between the two cases..

TestObject * _Nonnull returnsNilObjCInstanceDirectly() {
   // The first warning is from Sema. The second is from the static analyzer.
   return nil; // expected-warning {{null returned from function that requires a non-null return value}}
               // expected-warning@-1 {{nil returned from a function that is expected to return a non-null value}}
}
zaks.anna updated this revision to Diff 80925.Dec 9 2016, 11:43 AM
zaks.anna edited edge metadata.

Updated "null"-> "Null" as per Devin's suggestion.

dcoughlin accepted this revision.Dec 15 2016, 11:17 AM
dcoughlin edited edge metadata.

Looks good.

While you are here, you might consider changing: the checkBind() diagnostic to match the other diagnostics:

"Null assigned to a pointer which is expected to have non-null value" --> "Null assigned to a pointer which is expected to have *a* non-null value"

This revision is now accepted and ready to land.Dec 15 2016, 11:17 AM
This revision was automatically updated to reflect the committed changes.