[PATCH] D22392: [Sema] Fix an invalid nullability warning for binary conditional operators

[Akira Hatanaka via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: doug.gregor.
ahatanak added a subscriber: cfe-commits.

Currently clang issues a warning when the following code using a shorthand form of the conditional operator is compiled:

$ cat test1.c
```
void foo() {
  int * _Nullable p0;
  int * _Nonnull p1;
  int * _Nonnull p2 = p0 ?: p1;
}
```
test.c:4:23: warning: implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer
      type 'int * _Nonnull' [-Wnullable-to-nonnull-conversion]
  int * _Nonnull p2 = p0 ?: p1; 

This happens because sema uses the type of the first operand (p0's type, which is "int * _Nullable") for the type of the binary condition expression and then complains because a nullable pointer is being assigned to a nonnull pointer. The warning is not valid since the expression "p0 ?: p1" cannot be null if p1 is nonnull.

This patch fixes the bug by propagating the nullability of the last operand to the binary conditional expression.

https://reviews.llvm.org/D22392

Files:
  lib/Sema/SemaExpr.cpp
  test/Sema/nullability.c

Index: test/Sema/nullability.c
===================================================================
--- test/Sema/nullability.c
+++ test/Sema/nullability.c
@@ -128,3 +128,11 @@
 
   accepts_nonnull_1(ptr); // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
 }
+
+// Check nullability of binary conditional expressions.
+void modify_nullability(int c) {
+  int * _Nullable p0;
+  int * _Nonnull p1;
+
+  int * _Nonnull p2 = p0 ?: p1; // no warnings here.
+}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -7002,6 +7002,38 @@
     SourceRange(CondRHS->getLocStart(), RHSExpr->getLocEnd()));
 }
 
+/// Modify the nullability kind of the result type of a binary conditional
+/// expression.
+static QualType modifyNullability(QualType ResTy, Expr *RHSExpr,
+                                  ASTContext &Ctx) {
+  if (!ResTy->isPointerType())
+    return ResTy;
+
+  auto GetNullability = [&Ctx](QualType Ty) {
+    Optional<NullabilityKind> Kind = Ty->getNullability(Ctx);
+    if (Kind)
+      return *Kind;
+    return NullabilityKind::Unspecified;
+  };
+
+  // Change the result type only if the conditional expression is nullable and
+  // the RHS is nonnull.
+  NullabilityKind ResKind = GetNullability(ResTy);
+
+  if (GetNullability(ResTy) != NullabilityKind::Nullable)
+    return ResTy;
+
+  NullabilityKind NewKind = GetNullability(RHSExpr->getType());
+
+  if (NewKind != NullabilityKind::NonNull)
+    return ResTy;
+
+  // Create a new AttributedType with the new nullability kind.
+  QualType NewTy = ResTy.getDesugaredType(Ctx);
+  auto NewAttr = AttributedType::getNullabilityAttrKind(NewKind);
+  return Ctx.getAttributedType(NewAttr, NewTy, NewTy);
+}
+
 /// ActOnConditionalOp - Parse a ?: operation.  Note that 'LHS' may be null
 /// in the case of a the GNU conditional expr extension.
 ExprResult Sema::ActOnConditionalOp(SourceLocation QuestionLoc,
@@ -7088,6 +7120,7 @@
         ConditionalOperator(Cond.get(), QuestionLoc, LHS.get(), ColonLoc,
                             RHS.get(), result, VK, OK);
 
+  result = modifyNullability(result, RHSExpr, Context);
   return new (Context) BinaryConditionalOperator(
       commonExpr, opaqueValue, Cond.get(), LHS.get(), RHS.get(), QuestionLoc,
       ColonLoc, result, VK, OK);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D22392.64073.patch
Type: text/x-patch
Size: 2429 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160715/17eb1f28/attachment.bin>