This is an archive of the discontinued LLVM Phabricator instance.

[Static Analyzer] Minor cleanups for the nullability checker.
ClosedPublic

Authored by xazax.hun on Sep 3 2015, 5:21 PM.

Diff Detail

Event Timeline

xazax.hun updated this revision to Diff 33996.Sep 3 2015, 5:21 PM
xazax.hun retitled this revision from to [Static Analyzer] Minor cleanups for the nullability checker..
xazax.hun updated this object.
xazax.hun added reviewers: dcoughlin, zaks.anna.
xazax.hun added a subscriber: cfe-commits.

Why static was removed from getMostNullable() and getNullabilityString()? If these functions are not intended to be used outside this source file, it's good idea to limit their scope. Same should be done for ErrorMessages,

They are in the anonymous namespace. Maybe it would be cleaner to move them outside of the namespace and preserve the static keyword?

zaks.anna added inline comments.Sep 3 2015, 10:06 PM
lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
99

This is very asymmetric: "Nullable pointer" and "_Nonnull" type. Also, is "_Nonnull" the only spelling we have?

Overall.. LGTM

lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
78

After llvm_unreachable there is often not a return.

xazax.hun updated this revision to Diff 34458.Sep 10 2015, 10:16 AM
  • Updated to latest trunk
  • Reworded the diagnostic messages
zaks.anna accepted this revision.Sep 10 2015, 6:56 PM
zaks.anna edited edge metadata.
This revision is now accepted and ready to land.Sep 10 2015, 6:56 PM
This revision was automatically updated to reflect the committed changes.