ahatanak updated this revision to Diff 150218.
ahatanak marked an inline comment as done.
ahatanak added a reviewer: dcoughlin.
ahatanak set the repository for this revision to rC Clang.
ahatanak added a comment.

Sorry for the delay in responding. I've addressed Jordan's review comments.

I had to make changes to a couple of tests in Analysis. In particular, I'm not 
sure whether we should try to avoid producing extra diagnostics in 
test/Analysis/nullability_nullonly.mm or whether it's possible to do so in Sema.


Repository:
  rC Clang

https://reviews.llvm.org/D22391

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  test/Analysis/nullability-no-arc.mm
  test/Analysis/nullability.mm
  test/Analysis/nullability_nullonly.mm
  test/Sema/conditional-expr.c
  test/Sema/null_constant_to_nonnull.c

Index: test/Sema/null_constant_to_nonnull.c
===================================================================
--- /dev/null
+++ test/Sema/null_constant_to_nonnull.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -Wnullable-to-nonnull-conversion %s -verify
+
+void null_const_to_nonnull(int c) {
+  int * _Nonnull p0 = 0; // expected-warning{{implicitly casting a null constant to non-nullable pointer type 'int * _Nonnull'}}
+  p0 = 0; // expected-warning{{implicitly casting a null constant to non-nullable pointer type 'int * _Nonnull'}}
+  p0 = (int * _Nonnull)0; // explicit cast silences warnings
+  int * _Nonnull p1;
+  int * _Nonnull p2 = c ? p1 : 0; // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
+  p2 = c ? p1 : (int * _Nonnull)0; // explicit cast silences warnings
+}
Index: test/Sema/conditional-expr.c
===================================================================
--- test/Sema/conditional-expr.c
+++ test/Sema/conditional-expr.c
@@ -17,7 +17,7 @@
   dp = ip; // expected-warning {{incompatible pointer types assigning to 'double *' from 'int *'}}
   dp = 0 ? (double *)0 : (void *)0;
   vp = 0 ? (double *)0 : (void *)0;
-  ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer types assigning to 'int *' from 'double *'}}
+  ip = 0 ? (double *)0 : (void *)0; // expected-warning {{incompatible pointer types assigning to 'int *' from 'double * _Nullable'}}
 
   const int *cip;
   vp = (0 ? vp : cip); // expected-warning {{discards qualifiers}}
@@ -90,7 +90,7 @@
 
 int f0(int a) {
   // GCC considers this a warning.
-  return a ? f1() : nil; // expected-warning {{pointer/integer type mismatch in conditional expression ('int' and 'void *')}} expected-warning {{incompatible pointer to integer conversion returning 'void *' from a function with result type 'int'}}
+  return a ? f1() : nil; // expected-warning {{pointer/integer type mismatch in conditional expression ('int' and 'void *')}} expected-warning {{incompatible pointer to integer conversion returning 'void * _Nullable' from a function with result type 'int'}}
 }
 
 int f2(int x) {
Index: test/Analysis/nullability_nullonly.mm
===================================================================
--- test/Analysis/nullability_nullonly.mm
+++ test/Analysis/nullability_nullonly.mm
@@ -100,7 +100,7 @@
 }
 
 void testObjCARCExplicitZeroInitialization() {
-  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned to a pointer which is expected to have non-null value}}
+  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned to a pointer which is expected to have non-null value}} expected-warning{{implicitly casting a null constant to non-nullable pointer type 'TestObject * _Nonnull __strong'}}
 }
 
 // Under ARC, returned expressions of ObjC objects types are are implicitly
Index: test/Analysis/nullability.mm
===================================================================
--- test/Analysis/nullability.mm
+++ test/Analysis/nullability.mm
@@ -180,7 +180,7 @@
 
   // Since we've already had an invariant violation along this path,
   // we shouldn't warn here.
-  nonnullLocalWithAssignmentInInitializer = 0;
+  nonnullLocalWithAssignmentInInitializer = 0; // expected-warning {{implicitly casting a null constant to non-nullable pointer type}}
   (void)nonnullLocalWithAssignmentInInitializer;
 
 }
@@ -192,7 +192,7 @@
 
   // Since we've already had an invariant violation along this path,
   // we shouldn't warn here.
-  nonnullLocalWithAssignment = 0;
+  nonnullLocalWithAssignment = 0; // expected-warning {{implicitly casting a null constant to non-nullable pointer type}}
   (void)nonnullLocalWithAssignment;
 }
 
Index: test/Analysis/nullability-no-arc.mm
===================================================================
--- test/Analysis/nullability-no-arc.mm
+++ test/Analysis/nullability-no-arc.mm
@@ -43,7 +43,7 @@
 }
 
 void testObjCNonARCExplicitZeroInitialization() {
-  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned to a pointer which is expected to have non-null value}}
+  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{nil assigned to a pointer which is expected to have non-null value}} expected-warning {{implicitly casting a null constant to non-nullable pointer type 'TestObject * _Nonnull'}}
 }
 
 @interface ClassWithInitializers : NSObject
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -7157,20 +7157,27 @@
 }
 
 /// Compute the nullability of a conditional expression.
-static QualType computeConditionalNullability(QualType ResTy, bool IsBin,
-                                              QualType LHSTy, QualType RHSTy,
-                                              ASTContext &Ctx) {
+static QualType computeConditionalNullability(Sema &S, QualType ResTy,
+                                              bool IsBin, Expr *LHSExpr,
+                                              Expr *RHSExpr, ASTContext &Ctx) {
   if (!ResTy->isAnyPointerType())
     return ResTy;
 
-  auto GetNullability = [&Ctx](QualType Ty) {
+  auto GetNullability = [&S, &Ctx](QualType Ty, Expr *E = nullptr) {
+    // If E evaluates to a null constant and doesn't have a non-null type,
+    // return nullable.
+    if (E && S.CheckNonNullExpr(E))
+      return NullabilityKind::Nullable;
+
     Optional<NullabilityKind> Kind = Ty->getNullability(Ctx);
     if (Kind)
       return *Kind;
     return NullabilityKind::Unspecified;
   };
 
-  auto LHSKind = GetNullability(LHSTy), RHSKind = GetNullability(RHSTy);
+  QualType LHSTy = LHSExpr->getType(), RHSTy = RHSExpr->getType();
+  auto LHSKind = GetNullability(LHSTy, LHSExpr);
+  auto RHSKind = GetNullability(RHSTy, RHSExpr);
   NullabilityKind MergedKind;
 
   // Compute nullability of a binary conditional expression.
@@ -7282,7 +7289,6 @@
     LHSExpr = CondExpr = opaqueValue;
   }
 
-  QualType LHSTy = LHSExpr->getType(), RHSTy = RHSExpr->getType();
   ExprValueKind VK = VK_RValue;
   ExprObjectKind OK = OK_Ordinary;
   ExprResult Cond = CondExpr, LHS = LHSExpr, RHS = RHSExpr;
@@ -7297,8 +7303,8 @@
 
   CheckBoolLikeConversion(Cond.get(), QuestionLoc);
 
-  result = computeConditionalNullability(result, commonExpr, LHSTy, RHSTy,
-                                         Context);
+  result = computeConditionalNullability(*this, result, commonExpr, LHSExpr,
+                                         RHSExpr, Context);
 
   if (!commonExpr)
     return new (Context)
@@ -11094,6 +11100,9 @@
 
   CheckForNullPointerDereference(*this, LHSExpr);
 
+  if (const auto *DeclRef = dyn_cast<DeclRefExpr>(LHSExpr))
+    checkNullConstantToNonNull(DeclRef->getType(), RHS.get());
+
   // C99 6.5.16p3: The type of an assignment expression is the type of the
   // left operand unless the left operand has qualified type, in which case
   // it is the unqualified version of the type of the left operand.
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10771,6 +10771,8 @@
     Init = Result.getAs<Expr>();
   }
 
+  checkNullConstantToNonNull(DclT, Init);
+
   // Check for self-references within variable initializers.
   // Variables declared within a function/method body (except for references)
   // are handled by a dataflow analysis.
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2771,10 +2771,10 @@
 /// Checks if a the given expression evaluates to null.
 ///
 /// Returns true if the value evaluates to null.
-static bool CheckNonNullExpr(Sema &S, const Expr *Expr) {
+bool Sema::CheckNonNullExpr(const Expr *Expr) const {
   // If the expression has non-null type, it doesn't evaluate to null.
   if (auto nullability
-        = Expr->IgnoreImplicit()->getType()->getNullability(S.Context)) {
+        = Expr->IgnoreImplicit()->getType()->getNullability(Context)) {
     if (*nullability == NullabilityKind::NonNull)
       return false;
   }
@@ -2792,14 +2792,14 @@
 
   bool Result;
   return (!Expr->isValueDependent() &&
-          Expr->EvaluateAsBooleanCondition(Result, S.Context) &&
+          Expr->EvaluateAsBooleanCondition(Result, Context) &&
           !Result);
 }
 
 static void CheckNonNullArgument(Sema &S,
                                  const Expr *ArgExpr,
                                  SourceLocation CallSiteLoc) {
-  if (CheckNonNullExpr(S, ArgExpr))
+  if (S.CheckNonNullExpr(ArgExpr))
     S.DiagRuntimeBehavior(CallSiteLoc, ArgExpr,
            S.PDiag(diag::warn_null_arg) << ArgExpr->getSourceRange());
 }
@@ -8543,7 +8543,7 @@
   // Check if the return value is null but should not be.
   if (((Attrs && hasSpecificAttr<ReturnsNonNullAttr>(*Attrs)) ||
        (!isObjCMethod && isNonNullType(Context, lhsType))) &&
-      CheckNonNullExpr(*this, RetValExp))
+      CheckNonNullExpr(RetValExp))
     Diag(ReturnLoc, diag::warn_null_ret)
       << (isObjCMethod ? 1 : 0) << RetValExp->getSourceRange();
 
@@ -8558,13 +8558,18 @@
       const FunctionProtoType *Proto
         = FD->getType()->castAs<FunctionProtoType>();
       if (!Proto->isNothrow(/*ResultIfDependent*/true) &&
-          CheckNonNullExpr(*this, RetValExp))
+          CheckNonNullExpr(RetValExp))
         Diag(ReturnLoc, diag::warn_operator_new_returns_null)
           << FD << getLangOpts().CPlusPlus11;
     }
   }
 }
 
+void Sema::checkNullConstantToNonNull(QualType DstTy, Expr *RHSExpr) {
+  if (isNonNullType(Context, DstTy) && CheckNonNullExpr(RHSExpr))
+    Diag(RHSExpr->getLocStart(), diag::warn_null_constant_to_nonnull) << DstTy;
+}
+
 //===--- CHECK: Floating-Point comparisons (-Wfloat-equal) ---------------===//
 
 /// Check for comparisons of floating point operands using != and ==.
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -10444,6 +10444,10 @@
 
   static bool GetFormatNSStringIdx(const FormatAttr *Format, unsigned &Idx);
 
+  /// Returns true if the expression evaluates to null and doesn't have a
+  /// non-null type.
+  bool CheckNonNullExpr(const Expr *E) const;
+
 private:
   bool CheckFormatArguments(const FormatAttr *Format,
                             ArrayRef<const Expr *> Args,
@@ -10479,6 +10483,9 @@
                           const AttrVec *Attrs = nullptr,
                           const FunctionDecl *FD = nullptr);
 
+  /// Check null constant to nonnull conversion.
+  void checkNullConstantToNonNull(QualType DstTy, Expr *RHSExpr);
+
 public:
   void CheckFloatComparison(SourceLocation Loc, Expr *LHS, Expr *RHS);
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -9135,6 +9135,10 @@
   "zero as null pointer constant">,
   InGroup<DiagGroup<"zero-as-null-pointer-constant">>, DefaultIgnore;
 
+def warn_null_constant_to_nonnull : Warning<
+  "implicitly casting a null constant to non-nullable pointer type %0">,
+  InGroup<NullConstantToNonnull>;
+
 def err_nullability_cs_multilevel : Error<
   "nullability keyword %0 cannot be applied to multi-level pointer type %1">;
 def note_nullability_type_specifier : Note<
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -337,6 +337,7 @@
 def NullabilityDeclSpec : DiagGroup<"nullability-declspec">;
 def NullabilityInferredOnNestedType : DiagGroup<"nullability-inferred-on-nested-type">;
 def NullableToNonNullConversion : DiagGroup<"nullable-to-nonnull-conversion">;
+def NullConstantToNonnull : DiagGroup<"null-constant-to-nonnull">;
 def NullabilityCompletenessOnArrays : DiagGroup<"nullability-completeness-on-arrays">;
 def NullabilityCompleteness : DiagGroup<"nullability-completeness",
                                         [NullabilityCompletenessOnArrays]>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D22391: [Sema] Add ... Akira Hatanaka via Phabricator via cfe-commits

Reply via email to