[Sema] Propagate nullability when deducing type of auto

[ahatanak]

Summary

This patch fixes Sema to propagate the nullability of the initializer expression of a variable declared with auto or __auto_type to the deduced type. The patch consists of two parts:

• Define function QualType::setNullability, which is used to return a QualType with the specified nullability, and use it in computeConditionalNullability.
• Propagate nullability when type of auto is being deduced.

rdar://problem/27062504

[manmanren]

lib/Sema/SemaDecl.cpp

9739

Do we propagate other attributes for deduced types?

[rsmith]

Is this really a good idea? If I write:

int *_Nullable f();
int n;
int *_Nonnull g() {
  auto *p = f();
  if (!p) p = &n;
  return p;
}

... it would be wrong to produce a warning that I'm converting from a nullable pointer to a non-nullable pointer.

[ahatanak]

I didn't think of this case, but no, it isn't a good idea. We shouldn't issue a warning unless we know the returned pointer is nullable.

lib/Sema/SemaDecl.cpp

9739

QualType::setNullability can drop attributes on typedefs because it desugars the type until all nullability specifiers are stripped. I didn't try very hard to make sure QualType::setNullability preserve all the attributes because I couldn't come up with a case where dropping an attribute attached to a typedef for a pointer type would cause clang to mis-compile or issue a wrong diagnostic, so I thought it was safe.

[ahatanak]

Abandoning this revision.

It looks like we cannot correctly implement this warning without introducing a lot of complexity to clang. Users can use the static analyzer instead as it already warns about this case.

include/clang/AST/Type.h

QualType substObjCMemberType(QualType objectType,
                                ObjCSubstitutionContext context) const;
 
   /// Strip Objective-C "__kindof" types from the given type.
   QualType stripObjCKindOfType(const ASTContext &ctx) const;
 
   /// Remove all qualifiers including _Atomic.
   QualType getAtomicUnqualifiedType() const;
 
+  /// Create a type having the specified nullability.
+  QualType setNullability(Optional<NullabilityKind> Kind,
+                          ASTContext &Ctx) const;
+
 private:
   // These methods are implemented in a separate translation unit;
   // "static"-ize them to avoid creating temporary QualTypes in the
   // caller.
   static bool isConstant(QualType T, const ASTContext& Ctx);
   static QualType getDesugaredType(QualType T, const ASTContext &Context);
   static SplitQualType getSplitDesugaredType(QualType T);
   static SplitQualType getSplitUnqualifiedTypeImpl(QualType type);

lib/AST/Type.cpp

 }
 
 QualType QualType::getAtomicUnqualifiedType() const {
   if (auto AT = getTypePtr()->getAs<AtomicType>())
     return AT->getValueType().getUnqualifiedType();
   return getUnqualifiedType();
 }
 
+QualType QualType::setNullability(Optional<NullabilityKind> Kind,
+                                  ASTContext &Ctx) const {
+  assert(getTypePtr()->isAnyPointerType() && "type has to be a pointer type");
+  QualType ResTy = *this;
+
+  // Strip all nullability specifiers.
+  while (ResTy->getNullability(Ctx))
+    ResTy = ResTy.getSingleStepDesugaredType(Ctx);
+
+  if (!Kind)
+    return ResTy;
+
+  // Create a new AttributedType with the new nullability kind.
+  auto NewAttr = AttributedType::getNullabilityAttrKind(*Kind);
+  return Ctx.getAttributedType(NewAttr, ResTy, ResTy);
+}
+
 Optional<ArrayRef<QualType>> Type::getObjCSubstitutions(
                                const DeclContext *dc) const {
   // Look through method scopes.
   if (auto method = dyn_cast<ObjCMethodDecl>(dc))
     dc = method->getDeclContext();
 
   // Find the class or category in which the type we're substituting
   // was declared.

lib/Sema/SemaDecl.cpp

if (TypeMayContainAuto && VDecl->getType()->isUndeducedType()) {
     QualType DeducedType = deduceVarTypeFromInitializer(
         VDecl, VDecl->getDeclName(), VDecl->getType(),
         VDecl->getTypeSourceInfo(), VDecl->getSourceRange(), DirectInit, Init);
     if (DeducedType.isNull()) {
       RealDecl->setInvalidDecl();
       return;
     }
 
+    // Propagate init expression's nullability to the deduced type.
+    if (!Init->getType().isNull() && DeducedType->isAnyPointerType())
+      DeducedType = DeducedType.setNullability(
+          Init->getType()->getNullability(Context), Context);
+
     VDecl->setType(DeducedType);
     assert(VDecl->isLinkageValid());
 
     // In ARC, infer lifetime.
     if (getLangOpts().ObjCAutoRefCount && inferObjCARCLifetime(VDecl))
       VDecl->setInvalidDecl();
 
     // If this is a redeclaration, check that the type we just deduced matches

lib/Sema/SemaExpr.cpp

// Compute nullability of a normal conditional expression.
     else
       MergedKind = NullabilityKind::Unspecified;
   }
 
   // Return if ResTy already has the correct nullability.
   if (GetNullability(ResTy) == MergedKind)
     return ResTy;
 
-  // Strip all nullability from ResTy.
-  while (ResTy->getNullability(Ctx))
-    ResTy = ResTy.getSingleStepDesugaredType(Ctx);
-
-  // Create a new AttributedType with the new nullability kind.
-  auto NewAttr = AttributedType::getNullabilityAttrKind(MergedKind);
-  return Ctx.getAttributedType(NewAttr, ResTy, ResTy);
+  // Create a new type with the merged nullability kind.
+  return ResTy.setNullability(MergedKind, Ctx);
 }
 
 /// 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,
                                     SourceLocation ColonLoc,
                                     Expr *CondExpr, Expr *LHSExpr,
                                     Expr *RHSExpr) {

test/Sema/nullability.c

 void nullable_to_nonnull(_Nullable int *ptr) {
   int *a = ptr; // okay
   _Nonnull int *b = ptr; // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
   b = ptr; // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
 
   accepts_nonnull_1(ptr); // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
 }
 
+int * _Nullable return_nullable();
+
+int * _Nonnull deduce_auto() {
+  __auto_type *p = return_nullable();
+  return p; // expected-warning{{implicit conversion from nullable pointer 'int * _Nullable' to non-nullable pointer type 'int * _Nonnull'}}
+}
+
 // Check nullability of conditional expressions.
 void conditional_expr(int c) {
   int * _Nonnull p;
   int * _Nonnull nonnullP;
   int * _Nullable nullableP;
   int * _Null_unspecified unspecifiedP;
   int *noneP;
 

test/SemaCXX/nullability.cpp

void AssignAndInitNonNullFromFn() {
   void *_Nonnull p4 = ReturnNullable(); // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}}
   void *_Nonnull nonnull;
   nonnull = ReturnNullable(); // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}}
   nonnull = {ReturnNullable()}; // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}}
 
   TakeNonnull(ReturnNullable()); //expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull}}
 }
 
+void * _Nonnull DeduceAuto() {
+  auto *p = ReturnNullable();
+  return p; // expected-warning{{implicit conversion from nullable pointer 'void * _Nullable' to non-nullable pointer type 'void * _Nonnull'}}
+}
+
 void ConditionalExpr(bool c) {
   struct Base {};
   struct Derived : Base {};
 
   Base * _Nonnull p;
   Base * _Nonnull nonnullB;
   Base * _Nullable nullableB;
   Derived * _Nonnull nonnullD;