[Static Analyzer] Minor cleanups for the nullability checker.

[xazax.hun]

Summary

This patch contains minor cleanups and style fixes for the nullability checker. NFC.

[Eugene.Zelenko]

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,

[xazax.hun]

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]

lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

99 ↗

(On Diff #33996)

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

[danielmarjamaki]

Overall.. LGTM

lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

78 ↗

(On Diff #33996)

After llvm_unreachable there is often not a return.

[xazax.hun]

• Updated to latest trunk
• Reworded the diagnostic messages

cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

enum class Nullability : char {
   Nonnull
 };
 
 /// Returns the most nullable nullability. This is used for message expressions
 /// like [reciever method], where the nullability of this expression is either
 /// the nullability of the receiver or the nullability of the return type of the
 /// method, depending on which is more nullable. Contradicted is considered to
 /// be the most nullable, to avoid false positive results.
-static Nullability getMostNullable(Nullability Lhs, Nullability Rhs) {
+Nullability getMostNullable(Nullability Lhs, Nullability Rhs) {
   return static_cast<Nullability>(
       std::min(static_cast<char>(Lhs), static_cast<char>(Rhs)));
 }
 
-static const char *getNullabilityString(Nullability Nullab) {
+const char *getNullabilityString(Nullability Nullab) {
   switch (Nullab) {
   case Nullability::Contradicted:
     return "contradicted";
   case Nullability::Nullable:
     return "nullable";
   case Nullability::Unspecified:
     return "unspecified";
   case Nullability::Nonnull:
     return "nonnull";
   }
-  assert(false);
+  llvm_unreachable("Unexpected enumeration.");
   return "";
 }
 
 // These enums are used as an index to ErrorMessages array.
 enum class ErrorKind : int {
   NilAssignedToNonnull,
   NilPassedToNonnull,
   NilReturnedToNonnull,
   NullableAssignedToNonnull,
   NullableReturnedToNonnull,
   NullableDereferenced,
   NullablePassedToNonnull
 };
 
-const char *ErrorMessages[] = {"Null pointer is assigned to a pointer which "
-                               "has _Nonnull type",
-                               "Null pointer is passed to a parameter which is "
-                               "marked as _Nonnull",
-                               "Null pointer is returned from a function that "
-                               "has _Nonnull return type",
-                               "Nullable pointer is assigned to a pointer "
-                               "which has _Nonnull type",
-                               "Nullable pointer is returned from a function "
-                               "that has _Nonnull return type",
-                               "Nullable pointer is dereferenced",
-                               "Nullable pointer is passed to a parameter "
-                               "which is marked as _Nonnull"};
+const char *const ErrorMessages[] = {
+    "Null is assigned to a pointer which is expected to have non-null value",
+    "Null passed to a callee that requires a non-null argument",
+    "Null is returned from a function that is expected to return a non-null "
+    "value",
+    "Nullable pointer is assigned to a pointer which is expected to have "
+    "non-null value",
+    "Nullable pointer is returned from a function that is expected to return a "
+    "non-null value",
+    "Nullable pointer is passed to a calle that requires a non-null argument"};
 
 class NullabilityChecker
     : public Checker<check::Bind, check::PreCall, check::PreStmt<ReturnStmt>,
                      check::PostCall, check::PostStmt<ExplicitCastExpr>,
                      check::PostObjCMessage, check::DeadSymbols,
                      check::Event<ImplicitNullDerefEvent>> {
   mutable std::unique_ptr<BugType> BT;
 
void reportBugIfPreconditionHolds(ErrorKind Error, ExplodedNode *N,
                                     const Stmt *ValueExpr = nullptr,
                                     bool SuppressPath = false) const;
 
   void reportBug(ErrorKind Error, ExplodedNode *N, const MemRegion *Region,
                  BugReporter &BR, const Stmt *ValueExpr = nullptr) const {
     if (!BT)
       BT.reset(new BugType(this, "Nullability", "Memory error"));
     const char *Msg = ErrorMessages[static_cast<int>(Error)];
-    assert(Msg);
     std::unique_ptr<BugReport> R(new BugReport(*BT, Msg, N));
     if (Region) {
       R->markInteresting(Region);
       R->addVisitor(llvm::make_unique<NullabilityBugVisitor>(Region));
     }
     if (ValueExpr) {
       R->addRange(ValueExpr->getSourceRange());
       if (Error == ErrorKind::NilAssignedToNonnull ||
static const SymbolicRegion *getTrackRegion(SVal Val,
   if (!RegionSVal)
     return nullptr;
 
   const MemRegion *Region = RegionSVal->getRegion();
 
   if (CheckSuperRegion) {
     if (auto FieldReg = Region->getAs<FieldRegion>())
       return dyn_cast<SymbolicRegion>(FieldReg->getSuperRegion());
-    else if (auto ElementReg = Region->getAs<ElementRegion>())
+    if (auto ElementReg = Region->getAs<ElementRegion>())
       return dyn_cast<SymbolicRegion>(ElementReg->getSuperRegion());
   }
 
   return dyn_cast<SymbolicRegion>(Region);
 }
 
 PathDiagnosticPiece *NullabilityChecker::NullabilityBugVisitor::VisitNode(
     const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC,
     BugReport &BR) {
-  ProgramStateRef state = N->getState();
-  ProgramStateRef statePrev = PrevN->getState();
+  ProgramStateRef State = N->getState();
+  ProgramStateRef StatePrev = PrevN->getState();
 
-  const NullabilityState *TrackedNullab = state->get<NullabilityMap>(Region);
+  const NullabilityState *TrackedNullab = State->get<NullabilityMap>(Region);
   const NullabilityState *TrackedNullabPrev =
-      statePrev->get<NullabilityMap>(Region);
+      StatePrev->get<NullabilityMap>(Region);
   if (!TrackedNullab)
     return nullptr;
 
   if (TrackedNullabPrev &&
       TrackedNullabPrev->getValue() == TrackedNullab->getValue())
     return nullptr;
 
   // Retrieve the associated statement.
if (!TrackedNullability &&
       getNullabilityAnnotation(ReturnType) == Nullability::Nullable) {
     State = State->set<NullabilityMap>(Region, Nullability::Nullable);
     C.addTransition(State);
   }
 }
 
 static Nullability getReceiverNullability(const ObjCMethodCall &M,
                                           ProgramStateRef State) {
-  Nullability RetNullability = Nullability::Unspecified;
   if (M.isReceiverSelfOrSuper()) {
     // For super and super class receivers we assume that the receiver is
     // nonnull.
-    RetNullability = Nullability::Nonnull;
-  } else {
-    // Otherwise look up nullability in the state.
-    SVal Receiver = M.getReceiverSVal();
-    auto ValueRegionSVal = Receiver.getAs<loc::MemRegionVal>();
-    if (ValueRegionSVal) {
-      const MemRegion *SelfRegion = ValueRegionSVal->getRegion();
-      assert(SelfRegion);
+    return Nullability::Nonnull;
+  }
+  if (auto DefOrUnknown = Receiver.getAs<DefinedOrUnknownSVal>()) {
+    // If the receiver is constrained to be nonnull, assume that it is nonnull
+    // regardless of its type.
+    NullConstraint Nullness = getNullConstraint(*DefOrUnknown, State);
+    if (Nullness == NullConstraint::IsNotNull)
+      return Nullability::Nonnull;
+  }
 
-      const NullabilityState *TrackedSelfNullability =
-          State->get<NullabilityMap>(SelfRegion);
-      if (TrackedSelfNullability) {
-        RetNullability = TrackedSelfNullability->getValue();
-      }
-    }
-    if (auto DefOrUnknown = Receiver.getAs<DefinedOrUnknownSVal>()) {
-      // If the receiver is constrained to be nonnull, assume that it is nonnull
-      // regardless of its type.
-      NullConstraint Nullness = getNullConstraint(*DefOrUnknown, State);
-      if (Nullness == NullConstraint::IsNotNull)
-        RetNullability = Nullability::Nonnull;
-    }
-  }
-  return RetNullability;
+    if (TrackedSelfNullability)
+      return TrackedSelfNullability->getValue();
+  return Nullability::Unspecified;
 }
 
 /// Calculate the nullability of the result of a message expr based on the
 /// nullability of the receiver, the nullability of the return value, and the
 /// constraints.
 void NullabilityChecker::checkPostObjCMessage(const ObjCMethodCall &M,
                                               CheckerContext &C) const {
   auto Decl = M.getDecl();