From f68741b637c27ce2e7273207d3593759ba698a64 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 25 Jan 2024 11:57:01 +0000 Subject: [PATCH 1/6] Stop checking `err_count` in macro_rules validity checking All errors are local anyway, so we can track them directly --- compiler/rustc_expand/src/mbe/macro_rules.rs | 39 ++++++++++++-------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 1a39708ed8ed8..83e0f870c8a25 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -485,7 +485,9 @@ pub fn compile_declarative_macro( ) .pop() .unwrap(); - valid &= check_lhs_nt_follows(sess, def, &tt); + // We don't handle errors here, the driver will abort + // after parsing/expansion. we can report every error in every macro this way. + valid &= check_lhs_nt_follows(sess, def, &tt).is_ok(); return tt; } sess.dcx().span_bug(def.span, "wrong-structured lhs") @@ -589,18 +591,19 @@ pub fn compile_declarative_macro( (mk_syn_ext(expander), rule_spans) } -fn check_lhs_nt_follows(sess: &Session, def: &ast::Item, lhs: &mbe::TokenTree) -> bool { +fn check_lhs_nt_follows( + sess: &Session, + def: &ast::Item, + lhs: &mbe::TokenTree, +) -> Result<(), ErrorGuaranteed> { // lhs is going to be like TokenTree::Delimited(...), where the // entire lhs is those tts. Or, it can be a "bare sequence", not wrapped in parens. if let mbe::TokenTree::Delimited(.., delimited) = lhs { check_matcher(sess, def, &delimited.tts) } else { let msg = "invalid macro matcher; matchers must be contained in balanced delimiters"; - sess.dcx().span_err(lhs.span(), msg); - false + Err(sess.dcx().span_err(lhs.span(), msg)) } - // we don't abort on errors on rejection, the driver will do that for us - // after parsing/expansion. we can report every error in every macro this way. } fn is_empty_token_tree(sess: &Session, seq: &mbe::SequenceRepetition) -> bool { @@ -675,12 +678,15 @@ fn check_rhs(sess: &Session, rhs: &mbe::TokenTree) -> bool { false } -fn check_matcher(sess: &Session, def: &ast::Item, matcher: &[mbe::TokenTree]) -> bool { +fn check_matcher( + sess: &Session, + def: &ast::Item, + matcher: &[mbe::TokenTree], +) -> Result<(), ErrorGuaranteed> { let first_sets = FirstSets::new(matcher); let empty_suffix = TokenSet::empty(); - let err = sess.dcx().err_count(); - check_matcher_core(sess, def, &first_sets, matcher, &empty_suffix); - err == sess.dcx().err_count() + check_matcher_core(sess, def, &first_sets, matcher, &empty_suffix)?; + Ok(()) } fn has_compile_error_macro(rhs: &mbe::TokenTree) -> bool { @@ -1020,11 +1026,13 @@ fn check_matcher_core<'tt>( first_sets: &FirstSets<'tt>, matcher: &'tt [mbe::TokenTree], follow: &TokenSet<'tt>, -) -> TokenSet<'tt> { +) -> Result, ErrorGuaranteed> { use mbe::TokenTree; let mut last = TokenSet::empty(); + let mut errored = Ok(()); + // 2. For each token and suffix [T, SUFFIX] in M: // ensure that T can be followed by SUFFIX, and if SUFFIX may be empty, // then ensure T can also be followed by any element of FOLLOW. @@ -1068,7 +1076,7 @@ fn check_matcher_core<'tt>( token::CloseDelim(d.delim), span.close, )); - check_matcher_core(sess, def, first_sets, &d.tts, &my_suffix); + check_matcher_core(sess, def, first_sets, &d.tts, &my_suffix)?; // don't track non NT tokens last.replace_with_irrelevant(); @@ -1100,7 +1108,7 @@ fn check_matcher_core<'tt>( // At this point, `suffix_first` is built, and // `my_suffix` is some TokenSet that we can use // for checking the interior of `seq_rep`. - let next = check_matcher_core(sess, def, first_sets, &seq_rep.tts, my_suffix); + let next = check_matcher_core(sess, def, first_sets, &seq_rep.tts, my_suffix)?; if next.maybe_empty { last.add_all(&next); } else { @@ -1206,14 +1214,15 @@ fn check_matcher_core<'tt>( )); } } - err.emit(); + errored = Err(err.emit()); } } } } } } - last + errored?; + Ok(last) } fn token_can_be_followed_by_any(tok: &mbe::TokenTree) -> bool { From f6f0e04e9b9583fd3ba5a381d33894ebaa01d83f Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 25 Jan 2024 12:06:01 +0000 Subject: [PATCH 2/6] Remove an unused error count check --- compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs | 13 ------------- .../clippy/clippy_lints/src/transmute/utils.rs | 18 +----------------- 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs index 18f547be2a71c..b4c500206573e 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs @@ -45,14 +45,6 @@ pub struct FnCtxt<'a, 'tcx> { /// eventually). pub(super) param_env: ty::ParamEnv<'tcx>, - /// Number of errors that had been reported when we started - /// checking this function. On exit, if we find that *more* errors - /// have been reported, we will skip regionck and other work that - /// expects the types within the function to be consistent. - // FIXME(matthewjasper) This should not exist, and it's not correct - // if type checking is run in parallel. - err_count_on_creation: usize, - /// If `Some`, this stores coercion information for returned /// expressions. If `None`, this is in a context where return is /// inappropriate, such as a const expression. @@ -126,7 +118,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { FnCtxt { body_id, param_env, - err_count_on_creation: inh.tcx.dcx().err_count(), ret_coercion: None, ret_coercion_span: Cell::new(None), coroutine_types: None, @@ -195,10 +186,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }), } } - - pub fn errors_reported_since_creation(&self) -> bool { - self.dcx().err_count() > self.err_count_on_creation - } } impl<'a, 'tcx> Deref for FnCtxt<'a, 'tcx> { diff --git a/src/tools/clippy/clippy_lints/src/transmute/utils.rs b/src/tools/clippy/clippy_lints/src/transmute/utils.rs index 1cf6cf8548a64..7a7bb9f9c94c3 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/utils.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/utils.rs @@ -37,12 +37,6 @@ pub(super) fn check_cast<'tcx>( let inherited = Inherited::new(cx.tcx, local_def_id); let fn_ctxt = FnCtxt::new(&inherited, cx.param_env, local_def_id); - // If we already have errors, we can't be sure we can pointer cast. - assert!( - !fn_ctxt.errors_reported_since_creation(), - "Newly created FnCtxt contained errors" - ); - if let Ok(check) = cast::CastCheck::new( &fn_ctxt, e, @@ -53,17 +47,7 @@ pub(super) fn check_cast<'tcx>( DUMMY_SP, hir::Constness::NotConst, ) { - let res = check.do_check(&fn_ctxt); - - // do_check's documentation says that it might return Ok and create - // errors in the fcx instead of returning Err in some cases. Those cases - // should be filtered out before getting here. - assert!( - !fn_ctxt.errors_reported_since_creation(), - "`fn_ctxt` contained errors after cast check!" - ); - - res.ok() + check.do_check(&fn_ctxt).ok() } else { None } From 646c8fc2c1e9b4c8b67a21d8b40c820bd2aef7a7 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 25 Jan 2024 14:37:07 +0000 Subject: [PATCH 3/6] Statically ensure an error is reported in report_arg_errors --- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 6a77450f075bf..405ba3928420e 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -424,7 +424,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .map(|vars| self.resolve_vars_if_possible(vars)), ); - self.report_arg_errors( + self.set_tainted_by_errors(self.report_arg_errors( compatibility_diagonal, formal_and_expected_inputs, provided_args, @@ -433,7 +433,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn_def_id, call_span, call_expr, - ); + )); } } @@ -447,7 +447,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn_def_id: Option, call_span: Span, call_expr: &'tcx hir::Expr<'tcx>, - ) { + ) -> ErrorGuaranteed { // Next, let's construct the error let (error_span, full_call_span, call_name, is_method) = match &call_expr.kind { hir::ExprKind::Call( @@ -486,10 +486,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } let tcx = self.tcx; - // FIXME: taint after emitting errors and pass through an `ErrorGuaranteed` - self.set_tainted_by_errors( - tcx.dcx().span_delayed_bug(call_span, "no errors reported for args"), - ); // Get the argument span in the context of the call span so that // suggestions and labels are (more) correct when an arg is a @@ -696,8 +692,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Some(mismatch_idx), is_method, ); - err.emit(); - return; + return err.emit(); } } } @@ -721,11 +716,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if cfg!(debug_assertions) { span_bug!(error_span, "expected errors from argument matrix"); } else { - tcx.dcx().emit_err(errors::ArgMismatchIndeterminate { span: error_span }); + return tcx.dcx().emit_err(errors::ArgMismatchIndeterminate { span: error_span }); } - return; } + let mut reported = None; errors.retain(|error| { let Error::Invalid(provided_idx, expected_idx, Compatibility::Incompatible(Some(e))) = error @@ -736,16 +731,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let trace = mk_trace(provided_span, formal_and_expected_inputs[*expected_idx], provided_ty); if !matches!(trace.cause.as_failure_code(*e), FailureCode::Error0308) { - self.err_ctxt().report_and_explain_type_error(trace, *e).emit(); + reported = Some(self.err_ctxt().report_and_explain_type_error(trace, *e).emit()); return false; } true }); // We're done if we found errors, but we already emitted them. - if errors.is_empty() { - return; + if let Some(reported) = reported { + assert!(errors.is_empty()); + return reported; } + assert!(!errors.is_empty()); // Okay, now that we've emitted the special errors separately, we // are only left missing/extra/swapped and mismatched arguments, both @@ -802,8 +799,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Some(expected_idx.as_usize()), is_method, ); - err.emit(); - return; + return err.emit(); } let mut err = if formal_and_expected_inputs.len() == provided_args.len() { @@ -1251,7 +1247,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } - err.emit(); + err.emit() } fn suggest_ptr_null_mut( From 2b60e56e1f2ead042ce74b857bda05060963b604 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 25 Jan 2024 16:23:53 +0000 Subject: [PATCH 4/6] Statically ensure report_selection_error actually reports an error --- .../error_reporting/type_err_ctxt_ext.rs | 224 +++++++++--------- tests/ui/treat-err-as-bug/eagerly-emit.rs | 1 - tests/ui/treat-err-as-bug/eagerly-emit.stderr | 12 +- 3 files changed, 120 insertions(+), 117 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 149dcffe333de..ee2adc5a29ada 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -99,7 +99,7 @@ pub trait TypeErrCtxtExt<'tcx> { obligation: PredicateObligation<'tcx>, root_obligation: &PredicateObligation<'tcx>, error: &SelectionError<'tcx>, - ); + ) -> ErrorGuaranteed; fn emit_specialized_closure_kind_error( &self, @@ -208,10 +208,12 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } } + let mut reported = None; + for from_expansion in [false, true] { for (error, suppressed) in iter::zip(&errors, &is_suppressed) { if !suppressed && error.obligation.cause.span.from_expansion() == from_expansion { - self.report_fulfillment_error(error); + reported = Some(self.report_fulfillment_error(error)); // We want to ignore desugarings here: spans are equivalent even // if one is the result of a desugaring and the other is not. let mut span = error.obligation.cause.span; @@ -228,7 +230,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } } - self.dcx().delayed_bug("expected fulfillment errors") + // It could be that we don't report an error because we have seen an `ErrorReported` from another source. + // We should probably be able to fix most of these, but some are delayed bugs that get a proper error + // after this function. + reported.unwrap_or_else(|| self.dcx().delayed_bug("failed to report fulfillment errors")) } /// Reports that an overflow has occurred and halts compilation. We @@ -374,7 +379,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { mut obligation: PredicateObligation<'tcx>, root_obligation: &PredicateObligation<'tcx>, error: &SelectionError<'tcx>, - ) { + ) -> ErrorGuaranteed { let tcx = self.tcx; if tcx.sess.opts.unstable_opts.next_solver.map(|c| c.dump_tree).unwrap_or_default() @@ -384,10 +389,6 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } let mut span = obligation.cause.span; - // FIXME: statically guarantee this by tainting after the diagnostic is emitted - self.set_tainted_by_errors( - tcx.dcx().span_delayed_bug(span, "`report_selection_error` did not emit an error"), - ); let mut err = match *error { SelectionError::Unimplemented => { @@ -412,21 +413,19 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { kind: _, } = *obligation.cause.code() { - self.report_extra_impl_obligation( + return self.report_extra_impl_obligation( span, impl_item_def_id, trait_item_def_id, &format!("`{}`", obligation.predicate), ) - .emit(); - return; + .emit() } // Report a const-param specific error if let ObligationCauseCode::ConstParam(ty) = *obligation.cause.code().peel_derives() { - self.report_const_param_not_wf(ty, &obligation).emit(); - return; + return self.report_const_param_not_wf(ty, &obligation).emit(); } let bound_predicate = obligation.predicate.kind(); @@ -436,22 +435,22 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { let trait_predicate = self.resolve_vars_if_possible(trait_predicate); let trait_ref = trait_predicate.to_poly_trait_ref(); - if let Some(_guar) = self.emit_specialized_closure_kind_error(&obligation, trait_ref) { - return; + if let Some(guar) = self.emit_specialized_closure_kind_error(&obligation, trait_ref) { + return guar; } // FIXME(effects) let predicate_is_const = false; - if self.dcx().has_errors().is_some() + if let Some(guar) = self.dcx().has_errors() && trait_predicate.references_error() { - return; + return guar; } if self.fn_arg_obligation(&obligation) { // Silence redundant errors on binding acccess that are already // reported on the binding definition (#56607). - return; + return self.dcx().span_delayed_bug(obligation.cause.span, "error already reported"); } let mut file = None; let (post_message, pre_message, type_def) = self @@ -515,7 +514,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { trait_ref, span, ) { - GetSafeTransmuteErrorAndReason::Silent => return, + GetSafeTransmuteErrorAndReason::Silent => return self.dcx().span_delayed_bug(span, "silent safe transmute error"), GetSafeTransmuteErrorAndReason::Error { err_msg, safe_transmute_explanation, @@ -576,8 +575,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { have_alt_message, ) { self.note_obligation_cause(&mut err, &obligation); - err.emit(); - return; + return err.emit(); } file_note.map(|note| err.note(note)); @@ -680,13 +678,11 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } if self.suggest_add_clone_to_arg(&obligation, &mut err, trait_predicate) { - err.emit(); - return; + return err.emit(); } if self.suggest_impl_trait(&mut err, &obligation, trait_predicate) { - err.emit(); - return; + return err.emit(); } if is_unsize { @@ -776,8 +772,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { Some(sym::Debug | sym::Display) ) { - err.emit(); - return; + return err.emit(); } err @@ -912,8 +907,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { found_trait_ref, expected_trait_ref, ) { - Some(err) => err, - None => return, + Ok(err) => err, + Err(guar) => return guar, } } @@ -934,15 +929,15 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } SelectionError::NotConstEvaluatable(NotConstEvaluatable::MentionsParam) => { match self.report_not_const_evaluatable_error(&obligation, span) { - Some(err) => err, - None => return, + Ok(err) => err, + Err(guar) => return guar, } } // Already reported in the query. - SelectionError::NotConstEvaluatable(NotConstEvaluatable::Error(_)) | + SelectionError::NotConstEvaluatable(NotConstEvaluatable::Error(guar)) | // Already reported. - Overflow(OverflowError::Error(_)) => return, + Overflow(OverflowError::Error(guar)) => return guar, Overflow(_) => { bug!("overflow should be handled before the `report_selection_error` path"); @@ -951,7 +946,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { self.note_obligation_cause(&mut err, &obligation); self.point_at_returns_when_relevant(&mut err, &obligation); - err.emit(); + err.emit() } fn emit_specialized_closure_kind_error( @@ -1323,13 +1318,13 @@ pub(super) trait InferCtxtPrivExt<'tcx> { // `error` occurring implies that `cond` occurs. fn error_implies(&self, cond: ty::Predicate<'tcx>, error: ty::Predicate<'tcx>) -> bool; - fn report_fulfillment_error(&self, error: &FulfillmentError<'tcx>); + fn report_fulfillment_error(&self, error: &FulfillmentError<'tcx>) -> ErrorGuaranteed; fn report_projection_error( &self, obligation: &PredicateObligation<'tcx>, error: &MismatchedProjectionTypes<'tcx>, - ); + ) -> ErrorGuaranteed; fn maybe_detailed_projection_msg( &self, @@ -1395,7 +1390,7 @@ pub(super) trait InferCtxtPrivExt<'tcx> { trait_ref_and_ty: ty::Binder<'tcx, (ty::TraitPredicate<'tcx>, Ty<'tcx>)>, ) -> PredicateObligation<'tcx>; - fn maybe_report_ambiguity(&self, obligation: &PredicateObligation<'tcx>); + fn maybe_report_ambiguity(&self, obligation: &PredicateObligation<'tcx>) -> ErrorGuaranteed; fn predicate_can_apply( &self, @@ -1512,13 +1507,13 @@ pub(super) trait InferCtxtPrivExt<'tcx> { span: Span, found_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, expected_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, - ) -> Option>; + ) -> Result, ErrorGuaranteed>; fn report_not_const_evaluatable_error( &self, obligation: &PredicateObligation<'tcx>, span: Span, - ) -> Option>; + ) -> Result, ErrorGuaranteed>; } impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { @@ -1564,7 +1559,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } #[instrument(skip(self), level = "debug")] - fn report_fulfillment_error(&self, error: &FulfillmentError<'tcx>) { + fn report_fulfillment_error(&self, error: &FulfillmentError<'tcx>) -> ErrorGuaranteed { if self.tcx.sess.opts.unstable_opts.next_solver.map(|c| c.dump_tree).unwrap_or_default() == DumpSolverProofTree::OnError { @@ -1572,31 +1567,29 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } match error.code { - FulfillmentErrorCode::SelectionError(ref selection_error) => { - self.report_selection_error( + FulfillmentErrorCode::SelectionError(ref selection_error) => self + .report_selection_error( error.obligation.clone(), &error.root_obligation, selection_error, - ); - } + ), FulfillmentErrorCode::ProjectionError(ref e) => { - self.report_projection_error(&error.obligation, e); + self.report_projection_error(&error.obligation, e) } FulfillmentErrorCode::Ambiguity { overflow: false } => { - self.maybe_report_ambiguity(&error.obligation); + self.maybe_report_ambiguity(&error.obligation) } FulfillmentErrorCode::Ambiguity { overflow: true } => { - self.report_overflow_no_abort(error.obligation.clone()); + self.report_overflow_no_abort(error.obligation.clone()) } - FulfillmentErrorCode::SubtypeError(ref expected_found, ref err) => { - self.report_mismatched_types( + FulfillmentErrorCode::SubtypeError(ref expected_found, ref err) => self + .report_mismatched_types( &error.obligation.cause, expected_found.expected, expected_found.found, *err, ) - .emit(); - } + .emit(), FulfillmentErrorCode::ConstEquateError(ref expected_found, ref err) => { let mut diag = self.report_mismatched_consts( &error.obligation.cause, @@ -1620,11 +1613,9 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { &mut Default::default(), ); } - diag.emit(); - } - FulfillmentErrorCode::Cycle(ref cycle) => { - self.report_overflow_obligation_cycle(cycle); + diag.emit() } + FulfillmentErrorCode::Cycle(ref cycle) => self.report_overflow_obligation_cycle(cycle), } } @@ -1633,11 +1624,11 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { &self, obligation: &PredicateObligation<'tcx>, error: &MismatchedProjectionTypes<'tcx>, - ) { + ) -> ErrorGuaranteed { let predicate = self.resolve_vars_if_possible(obligation.predicate); - if predicate.references_error() { - return; + if let Err(e) = predicate.error_reported() { + return e; } self.probe(|_| { @@ -1802,8 +1793,8 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { false, ); self.note_obligation_cause(&mut diag, obligation); - diag.emit(); - }); + diag.emit() + }) } fn maybe_detailed_projection_msg( @@ -2341,7 +2332,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } #[instrument(skip(self), level = "debug")] - fn maybe_report_ambiguity(&self, obligation: &PredicateObligation<'tcx>) { + fn maybe_report_ambiguity(&self, obligation: &PredicateObligation<'tcx>) -> ErrorGuaranteed { // Unable to successfully determine, probably means // insufficient type information, but could mean // ambiguous impls. The latter *ought* to be a @@ -2361,8 +2352,8 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { let trait_ref = bound_predicate.rebind(data.trait_ref); debug!(?trait_ref); - if predicate.references_error() { - return; + if let Err(e) = predicate.error_reported() { + return e; } // This is kind of a hack: it frequently happens that some earlier @@ -2381,17 +2372,20 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { // check upstream for type errors and don't add the obligations to // begin with in those cases. if self.tcx.lang_items().sized_trait() == Some(trait_ref.def_id()) { - if let None = self.tainted_by_errors() { - let err = self.emit_inference_failure_err( - obligation.cause.body_id, - span, - trait_ref.self_ty().skip_binder().into(), - ErrorCode::E0282, - false, - ); - err.stash(span, StashKey::MaybeForgetReturn); + match self.tainted_by_errors() { + None => { + let err = self.emit_inference_failure_err( + obligation.cause.body_id, + span, + trait_ref.self_ty().skip_binder().into(), + ErrorCode::E0282, + false, + ); + err.stash(span, StashKey::MaybeForgetReturn); + return self.dcx().delayed_bug("stashed error never reported"); + } + Some(e) => return e, } - return; } // Typically, this ambiguity should only happen if @@ -2450,19 +2444,21 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } } if ambiguities.len() > 1 && ambiguities.len() < 10 && has_non_region_infer { - if self.tainted_by_errors().is_some() && subst.is_none() { + if let Some(e) = self.tainted_by_errors() + && subst.is_none() + { // If `subst.is_none()`, then this is probably two param-env // candidates or impl candidates that are equal modulo lifetimes. // Therefore, if we've already emitted an error, just skip this // one, since it's not particularly actionable. err.cancel(); - return; + return e; } self.annotate_source_of_ambiguity(&mut err, &ambiguities, predicate); } else { - if self.tainted_by_errors().is_some() { + if let Some(e) = self.tainted_by_errors() { err.cancel(); - return; + return e; } err.note(format!("cannot satisfy `{predicate}`")); let impl_candidates = self @@ -2605,11 +2601,15 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { ty::PredicateKind::Clause(ty::ClauseKind::WellFormed(arg)) => { // Same hacky approach as above to avoid deluging user // with error messages. - if arg.references_error() - || self.dcx().has_errors().is_some() - || self.tainted_by_errors().is_some() - { - return; + + if let Err(e) = arg.error_reported() { + return e; + } + if let Some(e) = self.tainted_by_errors() { + return e; + } + if let Some(e) = self.dcx().has_errors() { + return e; } self.emit_inference_failure_err( @@ -2622,12 +2622,15 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } ty::PredicateKind::Subtype(data) => { - if data.references_error() - || self.dcx().has_errors().is_some() - || self.tainted_by_errors().is_some() - { + if let Err(e) = data.error_reported() { + return e; + } + if let Some(e) = self.tainted_by_errors() { + return e; + } + if let Some(e) = self.dcx().has_errors() { // no need to overload user in such cases - return; + return e; } let SubtypePredicate { a_is_expected: _, a, b } = data; // both must be type variables, or the other would've been instantiated @@ -2641,8 +2644,11 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { ) } ty::PredicateKind::Clause(ty::ClauseKind::Projection(data)) => { - if predicate.references_error() || self.tainted_by_errors().is_some() { - return; + if let Err(e) = predicate.error_reported() { + return e; + } + if let Some(e) = self.tainted_by_errors() { + return e; } let subst = data .projection_ty @@ -2673,8 +2679,11 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } ty::PredicateKind::Clause(ty::ClauseKind::ConstEvaluatable(data)) => { - if predicate.references_error() || self.tainted_by_errors().is_some() { - return; + if let Err(e) = predicate.error_reported() { + return e; + } + if let Some(e) = self.tainted_by_errors() { + return e; } let subst = data.walk().find(|g| g.is_non_region_infer()); if let Some(subst) = subst { @@ -2699,8 +2708,12 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } } _ => { - if self.dcx().has_errors().is_some() || self.tainted_by_errors().is_some() { - return; + if let Some(e) = self.tainted_by_errors() { + return e; + } + if let Some(e) = self.dcx().has_errors() { + // no need to overload user in such cases + return e; } struct_span_code_err!( self.dcx(), @@ -2713,7 +2726,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } }; self.note_obligation_cause(&mut err, obligation); - err.emit(); + err.emit() } fn annotate_source_of_ambiguity( @@ -3433,16 +3446,14 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { span: Span, found_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, expected_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, - ) -> Option> { + ) -> Result, ErrorGuaranteed> { let found_trait_ref = self.resolve_vars_if_possible(found_trait_ref); let expected_trait_ref = self.resolve_vars_if_possible(expected_trait_ref); - if expected_trait_ref.self_ty().references_error() { - return None; - } + expected_trait_ref.self_ty().error_reported()?; let Some(found_trait_ty) = found_trait_ref.self_ty().no_bound_vars() else { - return None; + return Err(self.dcx().delayed_bug("bound vars outside binder")); }; let found_did = match *found_trait_ty.kind() { @@ -3456,7 +3467,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { if !self.reported_signature_mismatch.borrow_mut().insert((span, found_span)) { // We check closures twice, with obligations flowing in different directions, // but we want to complain about them only once. - return None; + return Err(self.dcx().span_delayed_bug(span, "already_reported")); } let mut not_tupled = false; @@ -3485,7 +3496,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { // This shouldn't be common unless manually implementing one of the // traits manually, but don't make it more confusing when it does // happen. - Some( + Ok( if Some(expected_trait_ref.def_id()) != self.tcx.lang_items().coroutine_trait() && not_tupled { @@ -3534,9 +3545,10 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { &self, obligation: &PredicateObligation<'tcx>, span: Span, - ) -> Option> { + ) -> Result, ErrorGuaranteed> { if !self.tcx.features().generic_const_exprs { - self.dcx() + let guar = self + .dcx() .struct_span_err(span, "constant expression depends on a generic parameter") // FIXME(const_generics): we should suggest to the user how they can resolve this // issue. However, this is currently not actually possible @@ -3546,7 +3558,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { // be reachable. .with_note("this may fail depending on what value the parameter takes") .emit(); - return None; + return Err(guar); } match obligation.predicate.kind().skip_binder() { @@ -3561,13 +3573,13 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { )), _ => err.help("consider adding a `where` bound using this expression"), }; - Some(err) + Ok(err) } ty::ConstKind::Expr(_) => { let err = self .dcx() .struct_span_err(span, format!("unconstrained generic constant `{ct}`")); - Some(err) + Ok(err) } _ => { bug!("const evaluatable failed for non-unevaluated const `{ct:?}`"); diff --git a/tests/ui/treat-err-as-bug/eagerly-emit.rs b/tests/ui/treat-err-as-bug/eagerly-emit.rs index 5f32f5a1d94b7..ede190575d529 100644 --- a/tests/ui/treat-err-as-bug/eagerly-emit.rs +++ b/tests/ui/treat-err-as-bug/eagerly-emit.rs @@ -6,6 +6,5 @@ fn main() {} fn f() -> impl Foo { //~^ ERROR the trait bound `i32: Foo` is not satisfied - //~| ERROR `report_selection_error` did not emit an error 1i32 } diff --git a/tests/ui/treat-err-as-bug/eagerly-emit.stderr b/tests/ui/treat-err-as-bug/eagerly-emit.stderr index 3d25741d52de8..4ae596435aa7f 100644 --- a/tests/ui/treat-err-as-bug/eagerly-emit.stderr +++ b/tests/ui/treat-err-as-bug/eagerly-emit.stderr @@ -1,9 +1,3 @@ -error: `report_selection_error` did not emit an error - --> $DIR/eagerly-emit.rs:7:11 - | -LL | fn f() -> impl Foo { - | ^^^^^^^^ - error: trimmed_def_paths constructed but no error emitted; use `DelayDm` for lints or `with_no_trimmed_paths` for debugging error[E0277]: the trait bound `i32: Foo` is not satisfied @@ -11,7 +5,7 @@ error[E0277]: the trait bound `i32: Foo` is not satisfied | LL | fn f() -> impl Foo { | ^^^^^^^^ the trait `Foo` is not implemented for `i32` -... +LL | LL | 1i32 | ---- return type was inferred to be `i32` here | @@ -21,8 +15,6 @@ help: this trait has no implementations, consider adding one LL | trait Foo {} | ^^^^^^^^^ -error: expected fulfillment errors - -error: aborting due to 4 previous errors +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0277`. From 3042da02485920f5a9d4c4149cc9e2a7d7ef69d0 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 25 Jan 2024 17:12:09 +0000 Subject: [PATCH 5/6] Remove has_errors check in builtin macro parsing --- compiler/rustc_builtin_macros/src/format.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index eb331ef585301..b66f7111ff006 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -139,7 +139,7 @@ fn parse_args<'a>(ecx: &mut ExtCtxt<'a>, sp: Span, tts: TokenStream) -> PResult< _ => { let expr = p.parse_expr()?; if !args.named_args().is_empty() { - ecx.dcx().emit_err(errors::PositionalAfterNamed { + return Err(ecx.dcx().create_err(errors::PositionalAfterNamed { span: expr.span, args: args .named_args() @@ -147,7 +147,7 @@ fn parse_args<'a>(ecx: &mut ExtCtxt<'a>, sp: Span, tts: TokenStream) -> PResult< .filter_map(|a| a.kind.ident().map(|ident| (a, ident))) .map(|(arg, n)| n.span.to(arg.expr.span)) .collect(), - }); + })); } args.add(FormatArgument { kind: FormatArgumentKind::Normal, expr }); } @@ -313,6 +313,8 @@ fn make_format_args( } use ArgRef::*; + let mut unnamed_arg_after_named_arg = false; + let mut lookup_arg = |arg: ArgRef<'_>, span: Option, used_as: PositionUsedAs, @@ -352,6 +354,7 @@ fn make_format_args( // For the moment capturing variables from format strings expanded from macros is // disabled (see RFC #2795) ecx.dcx().emit_err(errors::FormatNoArgNamed { span, name }); + unnamed_arg_after_named_arg = true; DummyResult::raw_expr(span, true) }; Ok(args.add(FormatArgument { kind: FormatArgumentKind::Captured(ident), expr })) @@ -510,7 +513,8 @@ fn make_format_args( }) .collect::>(); - if !unused.is_empty() { + let has_unused = !unused.is_empty(); + if has_unused { // If there's a lot of unused arguments, // let's check if this format arguments looks like another syntax (printf / shell). let detect_foreign_fmt = unused.len() > args.explicit_args().len() / 2; @@ -529,7 +533,7 @@ fn make_format_args( // Only check for unused named argument names if there are no other errors to avoid causing // too much noise in output errors, such as when a named argument is entirely unused. - if invalid_refs.is_empty() && ecx.dcx().has_errors().is_none() { + if invalid_refs.is_empty() && !has_unused && !unnamed_arg_after_named_arg { for &(index, span, used_as) in &numeric_refences_to_named_arg { let (position_sp_to_replace, position_sp_for_msg) = match used_as { Placeholder(pspan) => (span, pspan), From 054e1e3aada8ab0dd08af1f2f8e7ee16139d96e7 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 25 Jan 2024 17:19:12 +0000 Subject: [PATCH 6/6] Track ErrorGuaranteed instead of conjuring it from thin air --- compiler/rustc_infer/src/infer/mod.rs | 3 +- .../error_reporting/type_err_ctxt_ext.rs | 31 ++++++++++++------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 1eab8575fc0c4..0a39fe007fd22 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -278,7 +278,8 @@ pub struct InferCtxt<'tcx> { /// The set of predicates on which errors have been reported, to /// avoid reporting the same error twice. - pub reported_trait_errors: RefCell>>>, + pub reported_trait_errors: + RefCell>, ErrorGuaranteed)>>, pub reported_signature_mismatch: RefCell)>>, diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index ee2adc5a29ada..48084a41f251f 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -107,7 +107,10 @@ pub trait TypeErrCtxtExt<'tcx> { trait_ref: ty::PolyTraitRef<'tcx>, ) -> Option; - fn fn_arg_obligation(&self, obligation: &PredicateObligation<'tcx>) -> bool; + fn fn_arg_obligation( + &self, + obligation: &PredicateObligation<'tcx>, + ) -> Result<(), ErrorGuaranteed>; fn try_conversion_context( &self, @@ -142,6 +145,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { ( span, predicates + .0 .iter() .map(|&predicate| ErrorDescriptor { predicate, index: None }) .collect(), @@ -213,7 +217,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { for from_expansion in [false, true] { for (error, suppressed) in iter::zip(&errors, &is_suppressed) { if !suppressed && error.obligation.cause.span.from_expansion() == from_expansion { - reported = Some(self.report_fulfillment_error(error)); + let guar = self.report_fulfillment_error(error); + reported = Some(guar); // We want to ignore desugarings here: spans are equivalent even // if one is the result of a desugaring and the other is not. let mut span = error.obligation.cause.span; @@ -224,7 +229,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { self.reported_trait_errors .borrow_mut() .entry(span) - .or_default() + .or_insert_with(|| (vec![], guar)) + .0 .push(error.obligation.predicate); } } @@ -447,10 +453,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { { return guar; } - if self.fn_arg_obligation(&obligation) { - // Silence redundant errors on binding acccess that are already - // reported on the binding definition (#56607). - return self.dcx().span_delayed_bug(obligation.cause.span, "error already reported"); + // Silence redundant errors on binding acccess that are already + // reported on the binding definition (#56607). + if let Err(guar) = self.fn_arg_obligation(&obligation) { + return guar; } let mut file = None; let (post_message, pre_message, type_def) = self @@ -981,7 +987,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } } - fn fn_arg_obligation(&self, obligation: &PredicateObligation<'tcx>) -> bool { + fn fn_arg_obligation( + &self, + obligation: &PredicateObligation<'tcx>, + ) -> Result<(), ErrorGuaranteed> { if let ObligationCauseCode::FunctionArgumentObligation { arg_hir_id, .. } = obligation.cause.code() && let Some(Node::Expr(arg)) = self.tcx.opt_hir_node(*arg_hir_id) @@ -991,12 +1000,12 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { hir::Path { res: hir::def::Res::Local(hir_id), .. }, )) = arg.kind && let Some(Node::Pat(pat)) = self.tcx.opt_hir_node(*hir_id) - && let Some(preds) = self.reported_trait_errors.borrow().get(&pat.span) + && let Some((preds, guar)) = self.reported_trait_errors.borrow().get(&pat.span) && preds.contains(&obligation.predicate) { - return true; + return Err(*guar); } - false + Ok(()) } /// When the `E` of the resulting `Result` in an expression `foo().bar().baz()?`,