Add a clarifying note when a return statement is rejected because
we expect a related result type.

rdar://12493140

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@177378 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp
index 759f6c9..4bb5d72 100644
--- a/lib/Sema/SemaDeclObjC.cpp
+++ b/lib/Sema/SemaDeclObjC.cpp
@@ -151,7 +151,8 @@
     
     if (ObjCMethodFamily Family = Overridden->getMethodFamily())
       Diag(Overridden->getLocation(), 
-           diag::note_related_result_type_overridden_family)
+           diag::note_related_result_type_family)
+        << /*overridden method*/ 0
         << Family;
     else
       Diag(Overridden->getLocation(), 
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index bf4632c..e4323c3 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -10039,6 +10039,9 @@
 
   if (CheckInferredResultType)
     EmitRelatedResultTypeNote(SrcExpr);
+
+  if (Action == AA_Returning && ConvTy == IncompatiblePointer)
+    EmitRelatedResultTypeNoteForReturn(DstType);
   
   if (Complained)
     *Complained = true;
diff --git a/lib/Sema/SemaExprObjC.cpp b/lib/Sema/SemaExprObjC.cpp
index b0d5453..af8380d 100644
--- a/lib/Sema/SemaExprObjC.cpp
+++ b/lib/Sema/SemaExprObjC.cpp
@@ -1094,6 +1094,73 @@
   return ReceiverType;
 }
 
+/// Look for an ObjC method whose result type exactly matches the given type.
+static const ObjCMethodDecl *
+findExplicitInstancetypeDeclarer(const ObjCMethodDecl *MD,
+                                 QualType instancetype) {
+  if (MD->getResultType() == instancetype) return MD;
+
+  // For these purposes, a method in an @implementation overrides a
+  // declaration in the @interface.
+  if (const ObjCImplDecl *impl =
+        dyn_cast<ObjCImplDecl>(MD->getDeclContext())) {
+    const ObjCContainerDecl *iface;
+    if (const ObjCCategoryImplDecl *catImpl = 
+          dyn_cast<ObjCCategoryImplDecl>(impl)) {
+      iface = catImpl->getCategoryDecl();
+    } else {
+      iface = impl->getClassInterface();
+    }
+
+    const ObjCMethodDecl *ifaceMD = 
+      iface->getMethod(MD->getSelector(), MD->isInstanceMethod());
+    if (ifaceMD) return findExplicitInstancetypeDeclarer(ifaceMD, instancetype);
+  }
+
+  SmallVector<const ObjCMethodDecl *, 4> overrides;
+  MD->getOverriddenMethods(overrides);
+  for (unsigned i = 0, e = overrides.size(); i != e; ++i) {
+    if (const ObjCMethodDecl *result =
+          findExplicitInstancetypeDeclarer(overrides[i], instancetype))
+      return result;
+  }
+
+  return 0;
+}
+
+void Sema::EmitRelatedResultTypeNoteForReturn(QualType destType) {
+  // Only complain if we're in an ObjC method and the required return
+  // type doesn't match the method's declared return type.
+  ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(CurContext);
+  if (!MD || !MD->hasRelatedResultType() ||
+      Context.hasSameUnqualifiedType(destType, MD->getResultType()))
+    return;
+
+  // Look for a method overridden by this method which explicitly uses
+  // 'instancetype'.
+  if (const ObjCMethodDecl *overridden =
+        findExplicitInstancetypeDeclarer(MD, Context.getObjCInstanceType())) {
+    SourceLocation loc;
+    SourceRange range;
+    if (TypeSourceInfo *TSI = overridden->getResultTypeSourceInfo()) {
+      range = TSI->getTypeLoc().getSourceRange();
+      loc = range.getBegin();
+    }
+    if (loc.isInvalid())
+      loc = overridden->getLocation();
+    Diag(loc, diag::note_related_result_type_explicit)
+      << /*current method*/ 1 << range;
+    return;
+  }
+
+  // Otherwise, if we have an interesting method family, note that.
+  // This should always trigger if the above didn't.
+  if (ObjCMethodFamily family = MD->getMethodFamily())
+    Diag(MD->getLocation(), diag::note_related_result_type_family)
+      << /*current method*/ 1
+      << family;
+}
+
 void Sema::EmitRelatedResultTypeNote(const Expr *E) {
   E = E->IgnoreParenImpCasts();
   const ObjCMessageExpr *MsgSend = dyn_cast<ObjCMessageExpr>(E);
diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
index 2ddabbd..75aa17b 100644
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -5590,6 +5590,26 @@
 //===----------------------------------------------------------------------===//
 // Diagnose initialization failures
 //===----------------------------------------------------------------------===//
+
+/// Emit notes associated with an initialization that failed due to a
+/// "simple" conversion failure.
+static void emitBadConversionNotes(Sema &S, const InitializedEntity &entity,
+                                   Expr *op) {
+  QualType destType = entity.getType();
+  if (destType.getNonReferenceType()->isObjCObjectPointerType() &&
+      op->getType()->isObjCObjectPointerType()) {
+
+    // Emit a possible note about the conversion failing because the
+    // operand is a message send with a related result type.
+    S.EmitRelatedResultTypeNote(op);
+
+    // Emit a possible note about a return failing because we're
+    // expecting a related result type.
+    if (entity.getKind() == InitializedEntity::EK_Result)
+      S.EmitRelatedResultTypeNoteForReturn(destType);
+  }
+}
+
 bool InitializationSequence::Diagnose(Sema &S,
                                       const InitializedEntity &Entity,
                                       const InitializationKind &Kind,
@@ -5734,9 +5754,7 @@
       << Args[0]->isLValue()
       << Args[0]->getType()
       << Args[0]->getSourceRange();
-    if (DestType.getNonReferenceType()->isObjCObjectPointerType() &&
-        Args[0]->getType()->isObjCObjectPointerType())
-      S.EmitRelatedResultTypeNote(Args[0]);
+    emitBadConversionNotes(S, Entity, Args[0]);
     break;
 
   case FK_ConversionFailed: {
@@ -5749,9 +5767,7 @@
       << Args[0]->getSourceRange();
     S.HandleFunctionTypeMismatch(PDiag, FromType, DestType);
     S.Diag(Kind.getLocation(), PDiag);
-    if (DestType.getNonReferenceType()->isObjCObjectPointerType() &&
-        Args[0]->getType()->isObjCObjectPointerType())
-      S.EmitRelatedResultTypeNote(Args[0]);
+    emitBadConversionNotes(S, Entity, Args[0]);
     break;
   }
 
diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index b5b35fc..0c51e44 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -2536,20 +2536,7 @@
     if (!FnRetType->isDependentType() && !RetValExp->isTypeDependent()) {
       // we have a non-void function with an expression, continue checking
 
-      if (!RelatedRetType.isNull()) {
-        // If we have a related result type, perform an extra conversion here.
-        // FIXME: The diagnostics here don't really describe what is happening.
-        InitializedEntity Entity =
-            InitializedEntity::InitializeTemporary(RelatedRetType);
-
-        ExprResult Res = PerformCopyInitialization(Entity, SourceLocation(),
-                                                   RetValExp);
-        if (Res.isInvalid()) {
-          // FIXME: Cleanup temporaries here, anyway?
-          return StmtError();
-        }
-        RetValExp = Res.takeAs<Expr>();
-      }
+      QualType RetType = (RelatedRetType.isNull() ? FnRetType : RelatedRetType);
 
       // C99 6.8.6.4p3(136): The return statement is not an assignment. The
       // overlap restriction of subclause 6.5.16.1 does not apply to the case of
@@ -2559,18 +2546,33 @@
       // the C version of which boils down to CheckSingleAssignmentConstraints.
       NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, false);
       InitializedEntity Entity = InitializedEntity::InitializeResult(ReturnLoc,
-                                                                     FnRetType,
+                                                                     RetType,
                                                             NRVOCandidate != 0);
       ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRVOCandidate,
-                                                       FnRetType, RetValExp);
+                                                       RetType, RetValExp);
       if (Res.isInvalid()) {
-        // FIXME: Cleanup temporaries here, anyway?
+        // FIXME: Clean up temporaries here anyway?
         return StmtError();
       }
-
       RetValExp = Res.takeAs<Expr>();
-      if (RetValExp)
-        CheckReturnStackAddr(RetValExp, FnRetType, ReturnLoc);
+
+      // If we have a related result type, we need to implicitly
+      // convert back to the formal result type.  We can't pretend to
+      // initialize the result again --- we might end double-retaining
+      // --- so instead we initialize a notional temporary; this can
+      // lead to less-than-great diagnostics, but this stage is much
+      // less likely to fail than the previous stage.
+      if (!RelatedRetType.isNull()) {
+        Entity = InitializedEntity::InitializeTemporary(FnRetType);
+        Res = PerformCopyInitialization(Entity, ReturnLoc, RetValExp);
+        if (Res.isInvalid()) {
+          // FIXME: Clean up temporaries here anyway?
+          return StmtError();
+        }
+        RetValExp = Res.takeAs<Expr>();
+      }
+
+      CheckReturnStackAddr(RetValExp, FnRetType, ReturnLoc);
     }
 
     if (RetValExp) {