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);
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);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits