Rollup merge of #64746 - estebank:elide-impl-trait-obligations-on-err, r=cramertj

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 unneeded errors.

Fix #64084.

Author: Centril

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

@@ -664,12 +664,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

@@ -2698,30 +2698,39 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             traits::ObligationCause::new(span, self.body_id, code));
     }
 
-    pub fn require_type_is_sized(&self,
-                                 ty: Ty<'tcx>,
-                                 span: Span,
-                                 code: traits::ObligationCauseCode<'tcx>)
-    {
-        let lang_item = self.tcx.require_lang_item(lang_items::SizedTraitLangItem, None);
-        self.require_type_meets(ty, span, code, lang_item);
+    pub fn require_type_is_sized(
+        &self,
+        ty: Ty<'tcx>,
+        span: Span,
+        code: traits::ObligationCauseCode<'tcx>,
+    ) {
+        if !ty.references_error() {
+            let lang_item = self.tcx.require_lang_item(lang_items::SizedTraitLangItem, None);
+            self.require_type_meets(ty, span, code, lang_item);
+        }
     }
 
-    pub fn require_type_is_sized_deferred(&self,
-                                          ty: Ty<'tcx>,
-                                          span: Span,
-                                          code: traits::ObligationCauseCode<'tcx>)
-    {
-        self.deferred_sized_obligations.borrow_mut().push((ty, span, code));
+    pub fn require_type_is_sized_deferred(
+        &self,
+        ty: Ty<'tcx>,
+        span: Span,
+        code: traits::ObligationCauseCode<'tcx>,
+    ) {
+        if !ty.references_error() {
+            self.deferred_sized_obligations.borrow_mut().push((ty, span, code));
+        }
     }
 
-    pub fn register_bound(&self,
-                          ty: Ty<'tcx>,
-                          def_id: DefId,
-                          cause: traits::ObligationCause<'tcx>)
-    {
-        self.fulfillment_cx.borrow_mut()
-                           .register_bound(self, self.param_env, ty, def_id, cause);
+    pub fn register_bound(
+        &self,
+        ty: Ty<'tcx>,
+        def_id: DefId,
+        cause: traits::ObligationCause<'tcx>,
+    ) {
+        if !ty.references_error() {
+            self.fulfillment_cx.borrow_mut()
+                .register_bound(self, self.param_env, ty, def_id, cause);
+        }
     }
 
     pub fn to_ty(&self, ast_t: &hir::Ty) -> Ty<'tcx> {
@@ -2780,22 +2789,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
     /// Registers an obligation for checking later, during regionck, that the type `ty` must
     /// outlive the region `r`.
-    pub fn register_wf_obligation(&self,
-                                  ty: Ty<'tcx>,
-                                  span: Span,
-                                  code: traits::ObligationCauseCode<'tcx>)
-    {
+    pub fn register_wf_obligation(
+        &self,
+        ty: Ty<'tcx>,
+        span: Span,
+        code: traits::ObligationCauseCode<'tcx>,
+    ) {
         // WF obligations never themselves fail, so no real need to give a detailed cause:
         let cause = traits::ObligationCause::new(span, self.body_id, code);
-        self.register_predicate(traits::Obligation::new(cause,
-                                                        self.param_env,
-                                                        ty::Predicate::WellFormed(ty)));
+        self.register_predicate(
+            traits::Obligation::new(cause, self.param_env, ty::Predicate::WellFormed(ty)),
+        );
     }
 
     /// Registers obligations that all types appearing in `substs` are well-formed.
     pub fn add_wf_bounds(&self, substs: SubstsRef<'tcx>, expr: &hir::Expr) {
         for ty in substs.types() {
-            self.register_wf_obligation(ty, expr.span, traits::MiscObligation);
+            if !ty.references_error() {
+                self.register_wf_obligation(ty, expr.span, traits::MiscObligation);
+            }
         }
     }
 
@@ -2834,12 +2846,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     // FIXME(arielb1): use this instead of field.ty everywhere
     // Only for fields! Returns <none> for methods>
     // Indifferent to privacy flags
-    pub fn field_ty(&self,
-                    span: Span,
-                    field: &'tcx ty::FieldDef,
-                    substs: SubstsRef<'tcx>)
-                    -> Ty<'tcx>
-    {
+    pub fn field_ty(
+        &self,
+        span: Span,
+        field: &'tcx ty::FieldDef,
+        substs: SubstsRef<'tcx>,
+    ) -> Ty<'tcx> {
         self.normalize_associated_types_in(span, &field.ty(self.tcx, substs))
     }
 

src/librustc_typeck/check/wfcheck.rs

@@ -596,7 +596,7 @@ fn check_fn_or_method<'fcx, 'tcx>(
     }
     implied_bounds.extend(sig.inputs());
 
-    fcx.register_wf_obligation(sig.output(), span, ObligationCauseCode::MiscObligation);
+    fcx.register_wf_obligation(sig.output(), span, ObligationCauseCode::ReturnType);
 
     // FIXME(#25759) return types should not be implied bounds
     implied_bounds.push(sig.output());

src/test/ui/associated-types/associated-types-overridden-binding.rs

@@ -1,10 +1,10 @@
 #![feature(trait_alias)]
 
 trait Foo: Iterator<Item = i32> {}
-trait Bar: Foo<Item = u32> {} //~ ERROR type annotations required
+trait Bar: Foo<Item = u32> {} //~ ERROR type annotations needed
 
 trait I32Iterator = Iterator<Item = i32>;
-trait U32Iterator = I32Iterator<Item = u32>;
+trait U32Iterator = I32Iterator<Item = u32>; //~ ERROR type annotations needed
 
 fn main() {
     let _: &dyn I32Iterator<Item = u32>;