Remove blanket silencing of "type annotation needed" errors

Remove blanket check for existence of other errors before emitting
"type annotation needed" errors, and add some eager checks to avoid
adding obligations when they refer to types that reference
`[type error]` in order to reduce unneded errors.

Author: estebank

src/librustc/hir/lowering.rs

@@ -2184,9 +2184,7 @@ impl<'a> LoweringContext<'a> {
             match decl.output {
                 FunctionRetTy::Ty(ref ty) => match in_band_ty_params {
                     Some((def_id, _)) if impl_trait_return_allow => {
-                        hir::Return(self.lower_ty(ty,
-                            ImplTraitContext::OpaqueTy(Some(def_id))
-                        ))
+                        hir::Return(self.lower_ty(ty, ImplTraitContext::OpaqueTy(Some(def_id))))
                     }
                     _ => {
                         hir::Return(self.lower_ty(ty, ImplTraitContext::disallowed()))

src/librustc/infer/opaque_types/mod.rs

@@ -988,7 +988,9 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
         value.fold_with(&mut BottomUpFolder {
             tcx,
             ty_op: |ty| {
-                if let ty::Opaque(def_id, substs) = ty.sty {
+                if ty.references_error() {
+                    return tcx.types.err;
+                } else if let ty::Opaque(def_id, substs) = ty.sty {
                     // Check that this is `impl Trait` type is
                     // declared by `parent_def_id` -- i.e., one whose
                     // value we are inferring.  At present, this is
@@ -1155,6 +1157,15 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
         );
         debug!("instantiate_opaque_types: ty_var={:?}", ty_var);
 
+        for predicate in &bounds.predicates {
+            if let ty::Predicate::Projection(projection) = &predicate {
+                if projection.skip_binder().ty.references_error() {
+                    // No point on adding these obligations since there's a type error involved.
+                    return ty_var;
+                }
+            }
+        }
+
         self.obligations.reserve(bounds.predicates.len());
         for predicate in bounds.predicates {
             // Change the predicate to refer to the type variable,

src/librustc/traits/error_reporting.rs

@@ -1432,8 +1432,11 @@ impl<'tcx> TyCtxt<'tcx> {
 }
 
 impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
-    fn maybe_report_ambiguity(&self, obligation: &PredicateObligation<'tcx>,
-                              body_id: Option<hir::BodyId>) {
+    fn maybe_report_ambiguity(
+        &self,
+        obligation: &PredicateObligation<'tcx>,
+        body_id: Option<hir::BodyId>,
+    ) {
         // Unable to successfully determine, probably means
         // insufficient type information, but could mean
         // ambiguous impls. The latter *ought* to be a
@@ -1442,9 +1445,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         let predicate = self.resolve_vars_if_possible(&obligation.predicate);
         let span = obligation.cause.span;
 
-        debug!("maybe_report_ambiguity(predicate={:?}, obligation={:?})",
-               predicate,
-               obligation);
+        debug!(
+            "maybe_report_ambiguity(predicate={:?}, obligation={:?} body_id={:?}, code={:?})",
+            predicate,
+            obligation,
+            body_id,
+            obligation.cause.code,
+        );
 
         // Ambiguity errors are often caused as fallout from earlier
         // errors. So just ignore them if this infcx is tainted.
@@ -1456,6 +1463,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             ty::Predicate::Trait(ref data) => {
                 let trait_ref = data.to_poly_trait_ref();
                 let self_ty = trait_ref.self_ty();
+                debug!("self_ty {:?} {:?} trait_ref {:?}", self_ty, self_ty.sty, trait_ref);
+
                 if predicate.references_error() {
                     return;
                 }
@@ -1480,24 +1489,25 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                 // be ignoring the fact that we don't KNOW the type works
                 // out. Though even that would probably be harmless, given that
                 // we're only talking about builtin traits, which are known to be
-                // inhabited. But in any case I just threw in this check for
-                // has_errors() to be sure that compilation isn't happening
-                // anyway. In that case, why inundate the user.
-                if !self.tcx.sess.has_errors() {
-                    if
-                        self.tcx.lang_items().sized_trait()
-                        .map_or(false, |sized_id| sized_id == trait_ref.def_id())
-                    {
-                        self.need_type_info_err(body_id, span, self_ty).emit();
-                    } else {
-                        let mut err = struct_span_err!(self.tcx.sess,
-                                                       span, E0283,
-                                                       "type annotations required: \
-                                                        cannot resolve `{}`",
-                                                       predicate);
-                        self.note_obligation_cause(&mut err, obligation);
-                        err.emit();
-                    }
+                // inhabited. We used to check for `self.tcx.sess.has_errors()` to
+                // avoid inundating the user with unnecessary errors, but we now
+                // check upstream for type errors and dont add the obligations to
+                // begin with in those cases.
+                if
+                    self.tcx.lang_items().sized_trait()
+                    .map_or(false, |sized_id| sized_id == trait_ref.def_id())
+                {
+                    self.need_type_info_err(body_id, span, self_ty).emit();
+                } else {
+                    let mut err = struct_span_err!(
+                        self.tcx.sess,
+                        span,
+                        E0283,
+                        "type annotations needed: cannot resolve `{}`",
+                        predicate,
+                    );
+                    self.note_obligation_cause(&mut err, obligation);
+                    err.emit();
                 }
             }
 
@@ -1524,11 +1534,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
 
             _ => {
                 if !self.tcx.sess.has_errors() {
-                    let mut err = struct_span_err!(self.tcx.sess,
-                                                   obligation.cause.span, E0284,
-                                                   "type annotations required: \
-                                                    cannot resolve `{}`",
-                                                   predicate);
+                    let mut err = struct_span_err!(
+                        self.tcx.sess,
+                        obligation.cause.span,
+                        E0284,
+                        "type annotations needed: cannot resolve `{}`",
+                        predicate,
+                    );
                     self.note_obligation_cause(&mut err, obligation);
                     err.emit();
                 }
@@ -1766,7 +1778,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                               but not on the corresponding trait method",
                              predicate));
             }
-            ObligationCauseCode::ReturnType(_) |
+            ObligationCauseCode::ReturnType |
+            ObligationCauseCode::ReturnValue(_) |
             ObligationCauseCode::BlockTailExpression(_) => (),
             ObligationCauseCode::TrivialBound => {
                 err.help("see issue #48214");

src/librustc/traits/mod.rs

@@ -212,14 +212,14 @@ pub enum ObligationCauseCode<'tcx> {
     /// Constant expressions must be sized.
     ConstSized,
 
-    /// static items must have `Sync` type
+    /// Static items must have `Sync` type
     SharedStatic,
 
     BuiltinDerivedObligation(DerivedObligationCause<'tcx>),
 
     ImplDerivedObligation(DerivedObligationCause<'tcx>),
 
-    /// error derived when matching traits/impls; see ObligationCause for more details
+    /// Error derived when matching traits/impls; see ObligationCause for more details
     CompareImplMethodObligation {
         item_name: ast::Name,
         impl_item_def_id: DefId,
@@ -248,17 +248,20 @@ pub enum ObligationCauseCode<'tcx> {
     /// `start` has wrong type
     StartFunctionType,
 
-    /// intrinsic has wrong type
+    /// Intrinsic has wrong type
     IntrinsicType,
 
-    /// method receiver
+    /// Method receiver
     MethodReceiver,
 
     /// `return` with no expression
     ReturnNoExpression,
 
     /// `return` with an expression
-    ReturnType(hir::HirId),
+    ReturnValue(hir::HirId),
+
+    /// Return type of this function
+    ReturnType,
 
     /// Block implicit return
     BlockTailExpression(hir::HirId),

src/librustc/traits/select.rs

@@ -214,7 +214,7 @@ pub struct SelectionCache<'tcx> {
 /// of type variables - it just means the obligation isn't sufficiently
 /// elaborated. In that case we report an ambiguity, and the caller can
 /// try again after more type information has been gathered or report a
-/// "type annotations required" error.
+/// "type annotations needed" error.
 ///
 /// However, with type parameters, this can be a real problem - type
 /// parameters don't unify with regular types, but they *can* unify

src/librustc/traits/structural_impls.rs

@@ -485,7 +485,8 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
             super::TupleInitializerSized => Some(super::TupleInitializerSized),
             super::StructInitializerSized => Some(super::StructInitializerSized),
             super::VariableType(id) => Some(super::VariableType(id)),
-            super::ReturnType(id) => Some(super::ReturnType(id)),
+            super::ReturnValue(id) => Some(super::ReturnValue(id)),
+            super::ReturnType => Some(super::ReturnType),
             super::SizedArgumentType => Some(super::SizedArgumentType),
             super::SizedReturnType => Some(super::SizedReturnType),
             super::SizedYieldType => Some(super::SizedYieldType),

src/librustc/ty/mod.rs

@@ -438,19 +438,19 @@ bitflags! {
 
         /// `true` if there are "names" of types and regions and so forth
         /// that are local to a particular fn
-        const HAS_FREE_LOCAL_NAMES    = 1 << 9;
+        const HAS_FREE_LOCAL_NAMES = 1 << 9;
 
         /// Present if the type belongs in a local type context.
         /// Only set for Infer other than Fresh.
         const KEEP_IN_LOCAL_TCX  = 1 << 10;
 
         /// Does this have any `ReLateBound` regions? Used to check
         /// if a global bound is safe to evaluate.
-        const HAS_RE_LATE_BOUND = 1 << 11;
+        const HAS_RE_LATE_BOUND  = 1 << 11;
 
         const HAS_TY_PLACEHOLDER = 1 << 12;
 
-        const HAS_CT_INFER = 1 << 13;
+        const HAS_CT_INFER       = 1 << 13;
         const HAS_CT_PLACEHOLDER = 1 << 14;
 
         const NEEDS_SUBST        = TypeFlags::HAS_PARAMS.bits |

src/librustc_typeck/check/coercion.rs

@@ -1253,7 +1253,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
                             expression.map(|expr| (expr, blk_id)),
                         );
                     }
-                    ObligationCauseCode::ReturnType(id) => {
+                    ObligationCauseCode::ReturnValue(id) => {
                         db = self.report_return_mismatched_types(
                             cause, expected, found, err, fcx, id, None);
                     }

src/librustc_typeck/check/expr.rs

@@ -498,7 +498,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 expr.span,
                 infer::LateBoundRegionConversionTime::FnCall,
                 &fn_sig.output()).0;
-            self.require_type_is_sized_deferred(output, expr.span, traits::SizedReturnType);
+            if !fn_sig.output().references_error() {
+                self.require_type_is_sized_deferred(output, expr.span, traits::SizedReturnType);
+            }
         }
 
         // We always require that the type provided as the value for
@@ -664,12 +666,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         let ret_ty = ret_coercion.borrow().expected_ty();
         let return_expr_ty = self.check_expr_with_hint(return_expr, ret_ty.clone());
-        ret_coercion.borrow_mut()
-                    .coerce(self,
-                            &self.cause(return_expr.span,
-                                        ObligationCauseCode::ReturnType(return_expr.hir_id)),
-                            return_expr,
-                            return_expr_ty);
+        ret_coercion.borrow_mut().coerce(
+            self,
+            &self.cause(return_expr.span, ObligationCauseCode::ReturnValue(return_expr.hir_id)),
+            return_expr,
+            return_expr_ty,
+        );
     }
 
     /// Type check assignment expression `expr` of form `lhs = rhs`.

src/librustc_typeck/check/mod.rs

@@ -1097,7 +1097,9 @@ fn check_fn<'a, 'tcx>(
     *fcx.ps.borrow_mut() = UnsafetyState::function(fn_sig.unsafety, fn_id);
 
     let declared_ret_ty = fn_sig.output();
-    fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);
+    if !declared_ret_ty.references_error() {
+        fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);
+    }
     let revealed_ret_ty = fcx.instantiate_opaque_types_from_value(
         fn_id,
         &declared_ret_ty,