have coercion supply back the target type

The `try_coerce` method coerces from a source to a target
type, possibly inserting adjustments. It should guarantee
that the post-adjustment type is a subtype of the target type
(or else that some side-constraint has been registered which will lead
to an error). However, it used to return the (possibly adjusted) source
as the type of the expression rather than the target. This led to
less good downstream errors.

To work around this, the code around blocks -- and particular tail
expressions in blocks -- had some special case manipulation. However,
since that code is now using the more general `CoerceMany` construct (to
account for breaks), it can no longer take advantage of that. This lead
to some regressions in compile-fail tests were errors were reported at
"less good" locations than before.

This change modifies coercions to return the target type when successful
rather the source type. This extends the behavior from blocks to all
coercions. Typically this has limited effect but on a few tests yielded
better errors results (and avoided regressions, of course).

This change also restores the hint about removing semicolons which went
missing (by giving 'force-unit' coercions a chance to add notes etc).

7 files changed, 91 insertions, 61 deletions

diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs
index e83b786b9a8..4a044642444 100644
--- a/src/librustc_typeck/check/_match.rs
+++ b/src/librustc_typeck/check/_match.rs
@@ -498,7 +498,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             if is_if_let_fallback {
                 let cause = self.cause(expr.span, ObligationCauseCode::IfExpressionWithNoElse);
                 assert!(arm_ty.is_nil());
-                coercion.coerce_forced_unit(self, &cause);
+                coercion.coerce_forced_unit(self, &cause, &mut |_| ());
             } else {
                 let cause = self.cause(expr.span, ObligationCauseCode::MatchExpressionArm {
                     arm_span: arm.body.span,
diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs
index 37d442f1d05..a5acd0c7e53 100644
--- a/src/librustc_typeck/check/coercion.rs
+++ b/src/librustc_typeck/check/coercion.rs
@@ -74,6 +74,7 @@ use rustc::ty::fold::TypeFoldable;
 use rustc::ty::error::TypeError;
 use rustc::ty::relate::RelateResult;
 use rustc::ty::subst::Subst;
+use errors::DiagnosticBuilder;
 use syntax::abi;
 use syntax::feature_gate;
 use syntax::ptr::P;
@@ -718,7 +719,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 }
                 self.write_adjustment(expr.id, adjustment);
             }
-            Ok(adjustment.target)
+
+            // We should now have added sufficient adjustments etc to
+            // ensure that the type of expression, post-adjustment, is
+            // a subtype of target.
+            Ok(target)
         })
     }
 
@@ -1000,7 +1005,7 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E>
                       expression_ty: Ty<'tcx>,
                       expression_diverges: Diverges)
     {
-        self.coerce_inner(fcx, cause, Some(expression), expression_ty, expression_diverges)
+        self.coerce_inner(fcx, cause, Some(expression), expression_ty, expression_diverges, None)
     }
 
     /// Indicates that one of the inputs is a "forced unit". This
@@ -1011,15 +1016,21 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E>
     /// purposes. Note that these tend to correspond to cases where
     /// the `()` expression is implicit in the source, and hence we do
     /// not take an expression argument.
+    ///
+    /// The `augment_error` gives you a chance to extend the error
+    /// message, in case any results (e.g., we use this to suggest
+    /// removing a `;`).
     pub fn coerce_forced_unit<'a>(&mut self,
                                   fcx: &FnCtxt<'a, 'gcx, 'tcx>,
-                                  cause: &ObligationCause<'tcx>)
+                                  cause: &ObligationCause<'tcx>,
+                                  augment_error: &mut FnMut(&mut DiagnosticBuilder))
     {
         self.coerce_inner(fcx,
                           cause,
                           None,
                           fcx.tcx.mk_nil(),
-                          Diverges::Maybe)
+                          Diverges::Maybe,
+                          Some(augment_error))
     }
 
     /// The inner coercion "engine". If `expression` is `None`, this
@@ -1030,7 +1041,8 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E>
                         cause: &ObligationCause<'tcx>,
                         expression: Option<&'gcx hir::Expr>,
                         mut expression_ty: Ty<'tcx>,
-                        expression_diverges: Diverges)
+                        expression_diverges: Diverges,
+                        augment_error: Option<&mut FnMut(&mut DiagnosticBuilder)>)
     {
         // Incorporate whatever type inference information we have
         // until now; in principle we might also want to process
@@ -1126,19 +1138,25 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E>
                     (self.final_ty.unwrap_or(self.expected_ty), expression_ty)
                 };
 
+                let mut db;
                 match cause.code {
                     ObligationCauseCode::ReturnNoExpression => {
-                        struct_span_err!(fcx.tcx.sess, cause.span, E0069,
-                                         "`return;` in a function whose return type is not `()`")
-                            .span_label(cause.span, &format!("return type is not ()"))
-                            .emit();
+                        db = struct_span_err!(
+                            fcx.tcx.sess, cause.span, E0069,
+                            "`return;` in a function whose return type is not `()`");
+                        db.span_label(cause.span, &format!("return type is not ()"));
                     }
                     _ => {
-                        fcx.report_mismatched_types(cause, expected, found, err)
-                           .emit();
+                        db = fcx.report_mismatched_types(cause, expected, found, err);
                     }
                 }
 
+                if let Some(mut augment_error) = augment_error {
+                    augment_error(&mut db);
+                }
+
+                db.emit();
+
                 self.final_ty = Some(fcx.tcx.types.err);
             }
         }
diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
index 9eaa8404bde..29b83523aed 100644
--- a/src/librustc_typeck/check/mod.rs
+++ b/src/librustc_typeck/check/mod.rs
@@ -87,7 +87,7 @@ use fmt_macros::{Parser, Piece, Position};
 use hir::def::{Def, CtorKind};
 use hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
 use rustc_back::slice::ref_slice;
-use rustc::infer::{self, InferCtxt, InferOk, RegionVariableOrigin, TypeTrace};
+use rustc::infer::{self, InferCtxt, InferOk, RegionVariableOrigin};
 use rustc::infer::type_variable::{self, TypeVariableOrigin};
 use rustc::ty::subst::{Kind, Subst, Substs};
 use rustc::traits::{self, ObligationCause, ObligationCauseCode, Reveal};
@@ -99,6 +99,7 @@ use rustc::ty::adjustment;
 use rustc::ty::fold::{BottomUpFolder, TypeFoldable};
 use rustc::ty::maps::Providers;
 use rustc::ty::util::{Representability, IntTypeExt};
+use errors::DiagnosticBuilder;
 use require_c_abi_if_variadic;
 use session::{Session, CompileResult};
 use TypeAndSubsts;
@@ -2875,7 +2876,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             self.diverges.set(cond_diverges | then_diverges & else_diverges);
         } else {
             let else_cause = self.cause(sp, ObligationCauseCode::IfExpressionWithNoElse);
-            coerce.coerce_forced_unit(self, &else_cause);
+            coerce.coerce_forced_unit(self, &else_cause, &mut |_| ());
 
             // If the condition is false we can't diverge.
             self.diverges.set(cond_diverges);
@@ -3591,7 +3592,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                           coerce.coerce(self, &cause, e, e_ty, e_diverges);
                       } else {
                           assert!(e_ty.is_nil());
-                          coerce.coerce_forced_unit(self, &cause);
+                          coerce.coerce_forced_unit(self, &cause, &mut |_| ());
                       }
                   } else {
                       // If `ctxt.coerce` is `None`, we can just ignore
@@ -3626,7 +3627,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             } else {
                 let mut coercion = self.ret_coercion.as_ref().unwrap().borrow_mut();
                 let cause = self.cause(expr.span, ObligationCauseCode::ReturnNoExpression);
-                coercion.coerce_forced_unit(self, &cause);
+                coercion.coerce_forced_unit(self, &cause, &mut |_| ());
             }
             tcx.types.never
           }
@@ -4158,8 +4159,23 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                               tail_expr,
                               tail_expr_ty,
                               self.diverges.get()); // TODO
-            } else if !self.diverges.get().always() {
-                coerce.coerce_forced_unit(self, &self.misc(blk.span));
+            } else {
+                // Subtle: if there is no explicit tail expression,
+                // that is typically equivalent to a tail expression
+                // of `()` -- except if the block diverges. In that
+                // case, there is no value supplied from the tail
+                // expression (assuming there are no other breaks,
+                // this implies that the type of the block will be
+                // `!`).
+                if !self.diverges.get().always() {
+                    coerce.coerce_forced_unit(self, &self.misc(blk.span), &mut |err| {
+                        if let Some(expected_ty) = expected.only_has_type(self) {
+                            self.consider_hint_about_removing_semicolon(blk,
+                                                                        expected_ty,
+                                                                        err);
+                        }
+                    });
+                }
             }
         });
 
@@ -4175,43 +4191,42 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         ty
     }
 
-    pub fn check_block_no_expr(&self, blk: &'gcx hir::Block, ty: Ty<'tcx>, ety: Ty<'tcx>) {
-        // We're not diverging and there's an expected type, which,
-        // in case it's not `()`, could result in an error higher-up.
-        // We have a chance to error here early and be more helpful.
-        let cause = self.misc(blk.span);
-        let trace = TypeTrace::types(&cause, false, ty, ety);
-        match self.sub_types(false, &cause, ty, ety) {
-            Ok(InferOk { obligations, .. }) => {
-                // FIXME(#32730) propagate obligations
-                assert!(obligations.is_empty());
-            },
-            Err(err) => {
-                let mut err = self.report_and_explain_type_error(trace, &err);
-
-                // Be helpful when the user wrote `{... expr;}` and
-                // taking the `;` off is enough to fix the error.
-                let mut extra_semi = None;
-                if let Some(stmt) = blk.stmts.last() {
-                    if let hir::StmtSemi(ref e, _) = stmt.node {
-                        if self.can_sub_types(self.node_ty(e.id), ety).is_ok() {
-                            extra_semi = Some(stmt);
-                        }
-                    }
-                }
-                if let Some(last_stmt) = extra_semi {
-                    let original_span = original_sp(last_stmt.span, blk.span);
-                    let span_semi = Span {
-                        lo: original_span.hi - BytePos(1),
-                        hi: original_span.hi,
-                        ctxt: original_span.ctxt,
-                    };
-                    err.span_help(span_semi, "consider removing this semicolon:");
-                }
-
-                err.emit();
-            }
+    /// A common error is to add an extra semicolon:
+    ///
+    /// ```
+    /// fn foo() -> usize {
+    ///     22;
+    /// }
+    /// ```
+    ///
+    /// This routine checks if the final statement in a block is an
+    /// expression with an explicit semicolon whose type is compatible
+    /// with `expected_ty`. If so, it suggests removing the semicolon.
+    fn consider_hint_about_removing_semicolon(&self,
+                                              blk: &'gcx hir::Block,
+                                              expected_ty: Ty<'tcx>,
+                                              err: &mut DiagnosticBuilder) {
+        // Be helpful when the user wrote `{... expr;}` and
+        // taking the `;` off is enough to fix the error.
+        let last_stmt = match blk.stmts.last() {
+            Some(s) => s,
+            None => return,
+        };
+        let last_expr = match last_stmt.node {
+            hir::StmtSemi(ref e, _) => e,
+            _ => return,
+        };
+        let last_expr_ty = self.expr_ty(last_expr);
+        if self.can_sub_types(last_expr_ty, expected_ty).is_err() {
+            return;
         }
+        let original_span = original_sp(last_stmt.span, blk.span);
+        let span_semi = Span {
+            lo: original_span.hi - BytePos(1),
+            hi: original_span.hi,
+            ctxt: original_span.ctxt,
+        };
+        err.span_help(span_semi, "consider removing this semicolon:");
     }
 
     // Instantiates the given path, which must refer to an item with the given
diff --git a/src/test/compile-fail/issue-15965.rs b/src/test/compile-fail/issue-15965.rs
index d5d597c190e..08b896f387b 100644
--- a/src/test/compile-fail/issue-15965.rs
+++ b/src/test/compile-fail/issue-15965.rs
@@ -11,7 +11,7 @@
 fn main() {
     return
         { return () }
-//~^ ERROR expected function, found `!`
+//~^ ERROR the type of this value must be known in this context
     ()
     ;
 }
diff --git a/src/test/compile-fail/issue-2149.rs b/src/test/compile-fail/issue-2149.rs
index 9143a226a24..256c5d8e6f7 100644
--- a/src/test/compile-fail/issue-2149.rs
+++ b/src/test/compile-fail/issue-2149.rs
@@ -21,5 +21,5 @@ impl<A> vec_monad<A> for Vec<A> {
 }
 fn main() {
     ["hi"].bind(|x| [x] );
-    //~^ ERROR no method named `bind` found for type `[&'static str; 1]` in the current scope
+    //~^ ERROR no method named `bind` found for type `[&str; 1]` in the current scope
 }
diff --git a/src/test/compile-fail/region-invariant-static-error-reporting.rs b/src/test/compile-fail/region-invariant-static-error-reporting.rs
index ac0167e08bd..d25674a74b1 100644
--- a/src/test/compile-fail/region-invariant-static-error-reporting.rs
+++ b/src/test/compile-fail/region-invariant-static-error-reporting.rs
@@ -16,9 +16,6 @@
 // error-pattern:cannot infer
 // error-pattern:cannot outlive the lifetime 'a
 // error-pattern:must be valid for the static lifetime
-// error-pattern:cannot infer
-// error-pattern:cannot outlive the lifetime 'a
-// error-pattern:must be valid for the static lifetime
 
 struct Invariant<'a>(Option<&'a mut &'a mut ()>);
 
diff --git a/src/test/compile-fail/regions-bounds.rs b/src/test/compile-fail/regions-bounds.rs
index 810a8671c53..5ce80be98d9 100644
--- a/src/test/compile-fail/regions-bounds.rs
+++ b/src/test/compile-fail/regions-bounds.rs
@@ -16,11 +16,11 @@ struct an_enum<'a>(&'a isize);
 struct a_class<'a> { x:&'a isize }
 
 fn a_fn1<'a,'b>(e: an_enum<'a>) -> an_enum<'b> {
-    return e; //~^ ERROR mismatched types
+    return e; //~ ERROR mismatched types
 }
 
 fn a_fn3<'a,'b>(e: a_class<'a>) -> a_class<'b> {
-    return e; //~^ ERROR mismatched types
+    return e; //~ ERROR mismatched types
 }
 
 fn main() { }