From d0d40d2a4073b17a14f364dc6638fea2faecbc2b Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Wed, 19 Apr 2023 14:59:36 -0700 Subject: [PATCH 1/9] Break up long function in trait selection error reporting - Move blocks of code into their own functions - Replace a few function argument types with their type aliases --- compiler/rustc_hir_typeck/src/method/mod.rs | 3 +- compiler/rustc_infer/src/traits/mod.rs | 2 +- .../src/traits/auto_trait.rs | 2 +- .../src/traits/error_reporting/mod.rs | 1077 ++++++++++------- .../error_reporting/on_unimplemented.rs | 1 + .../src/traits/error_reporting/suggestions.rs | 4 +- 6 files changed, 659 insertions(+), 430 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/method/mod.rs b/compiler/rustc_hir_typeck/src/method/mod.rs index 0456dd56c34..fab9a8a5f4f 100644 --- a/compiler/rustc_hir_typeck/src/method/mod.rs +++ b/compiler/rustc_hir_typeck/src/method/mod.rs @@ -300,8 +300,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { trait_def_id: DefId, self_ty: Ty<'tcx>, opt_input_types: Option<&[Ty<'tcx>]>, - ) -> (traits::Obligation<'tcx, ty::Predicate<'tcx>>, &'tcx ty::List>) - { + ) -> (traits::PredicateObligation<'tcx>, &'tcx ty::List>) { // Construct a trait-reference `self_ty : Trait` let substs = InternalSubsts::for_item(self.tcx, trait_def_id, |param, _| { match param.kind { diff --git a/compiler/rustc_infer/src/traits/mod.rs b/compiler/rustc_infer/src/traits/mod.rs index 9dd4f0a8e4c..8ce8b4e2024 100644 --- a/compiler/rustc_infer/src/traits/mod.rs +++ b/compiler/rustc_infer/src/traits/mod.rs @@ -123,7 +123,7 @@ pub struct FulfillmentError<'tcx> { #[derive(Clone)] pub enum FulfillmentErrorCode<'tcx> { /// Inherently impossible to fulfill; this trait is implemented if and only if it is already implemented. - CodeCycle(Vec>>), + CodeCycle(Vec>), CodeSelectionError(SelectionError<'tcx>), CodeProjectionError(MismatchedProjectionTypes<'tcx>), CodeSubtypeError(ExpectedFound>, TypeError<'tcx>), // always comes from a SubtypePredicate diff --git a/compiler/rustc_trait_selection/src/traits/auto_trait.rs b/compiler/rustc_trait_selection/src/traits/auto_trait.rs index 222af4c2ace..e82672e8368 100644 --- a/compiler/rustc_trait_selection/src/traits/auto_trait.rs +++ b/compiler/rustc_trait_selection/src/traits/auto_trait.rs @@ -591,7 +591,7 @@ impl<'tcx> AutoTraitFinder<'tcx> { fn evaluate_nested_obligations( &self, ty: Ty<'_>, - nested: impl Iterator>>, + nested: impl Iterator>, computed_preds: &mut FxIndexSet>, fresh_preds: &mut FxHashSet>, predicates: &mut VecDeque>, diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index ae21dcd2a36..87b6092bd5d 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -713,29 +713,13 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { (message, note, append_const_msg) }; - let err_msg = message - .and_then(|cannot_do_this| { - match (predicate_is_const, append_const_msg) { - // do nothing if predicate is not const - (false, _) => Some(cannot_do_this), - // suggested using default post message - (true, Some(None)) => { - Some(format!("{cannot_do_this} in const contexts")) - } - // overridden post message - (true, Some(Some(post_message))) => { - Some(format!("{cannot_do_this}{post_message}")) - } - // fallback to generic message - (true, None) => None, - } - }) - .unwrap_or_else(|| { - format!( - "the trait bound `{}` is not satisfied{}", - trait_predicate, post_message, - ) - }); + let err_msg = self.get_standard_error_message( + &trait_predicate, + message, + predicate_is_const, + append_const_msg, + post_message, + ); let (err_msg, safe_transmute_explanation) = if Some(trait_ref.def_id()) == self.tcx.lang_items().transmute_trait() @@ -763,22 +747,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } if Some(trait_ref.def_id()) == tcx.lang_items().tuple_trait() { - match obligation.cause.code().peel_derives() { - ObligationCauseCode::RustCall => { - err.set_primary_message("functions with the \"rust-call\" ABI must take a single non-self tuple argument"); - } - ObligationCauseCode::BindingObligation(def_id, _) - | ObligationCauseCode::ItemObligation(def_id) - if tcx.is_fn_trait(*def_id) => - { - err.code(rustc_errors::error_code!(E0059)); - err.set_primary_message(format!( - "type parameter to bare `{}` trait must be a tuple", - tcx.def_path_str(*def_id) - )); - } - _ => {} - } + self.add_tuple_trait_message( + &obligation.cause.code().peel_derives(), + &mut err, + ); } if Some(trait_ref.def_id()) == tcx.lang_items().drop_trait() @@ -788,33 +760,13 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { err.note("See for more details"); } - let explanation = if let ObligationCauseCode::MainFunctionType = - obligation.cause.code() - { - "consider using `()`, or a `Result`".to_owned() - } else { - let ty_desc = match trait_ref.skip_binder().self_ty().kind() { - ty::FnDef(_, _) => Some("fn item"), - ty::Closure(_, _) => Some("closure"), - _ => None, - }; + let explanation = get_explanation_based_on_obligation( + &obligation, + trait_ref, + &trait_predicate, + pre_message, + ); - match ty_desc { - Some(desc) => format!( - "{}the trait `{}` is not implemented for {} `{}`", - pre_message, - trait_predicate.print_modifiers_and_trait_path(), - desc, - trait_ref.skip_binder().self_ty(), - ), - None => format!( - "{}the trait `{}` is not implemented for `{}`", - pre_message, - trait_predicate.print_modifiers_and_trait_path(), - trait_ref.skip_binder().self_ty(), - ), - } - }; self.check_for_binding_assigned_block_without_tail_expression( &obligation, &mut err, @@ -851,28 +803,14 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { self.suggest_borrowing_for_object_cast(&mut err, &root_obligation, *concrete_ty, *obj_ty); } - let mut unsatisfied_const = false; - if trait_predicate.is_const_if_const() && obligation.param_env.is_const() { - let non_const_predicate = trait_ref.without_const(); - let non_const_obligation = Obligation { - cause: obligation.cause.clone(), - param_env: obligation.param_env.without_const(), - predicate: non_const_predicate.to_predicate(tcx), - recursion_depth: obligation.recursion_depth, - }; - if self.predicate_may_hold(&non_const_obligation) { - unsatisfied_const = true; - err.span_note( - span, - &format!( - "the trait `{}` is implemented for `{}`, \ - but that implementation is not `const`", - non_const_predicate.print_modifiers_and_trait_path(), - trait_ref.skip_binder().self_ty(), - ), - ); - } - } + let UnsatisfiedConst(unsatisfied_const) = self + .maybe_add_note_for_unsatisfied_const( + &obligation, + trait_ref, + &trait_predicate, + &mut err, + span, + ); if let Some((msg, span)) = type_def { err.span_label(span, &msg); @@ -970,137 +908,16 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { ); } - let body_def_id = obligation.cause.body_id; - // Try to report a help message - if is_fn_trait - && let Ok((implemented_kind, params)) = self.type_implements_fn_trait( - obligation.param_env, - trait_ref.self_ty(), - trait_predicate.skip_binder().constness, - trait_predicate.skip_binder().polarity, - ) - { - // If the type implements `Fn`, `FnMut`, or `FnOnce`, suppress the following - // suggestion to add trait bounds for the type, since we only typically implement - // these traits once. - - // Note if the `FnMut` or `FnOnce` is less general than the trait we're trying - // to implement. - let selected_kind = - self.tcx.fn_trait_kind_from_def_id(trait_ref.def_id()) - .expect("expected to map DefId to ClosureKind"); - if !implemented_kind.extends(selected_kind) { - err.note( - &format!( - "`{}` implements `{}`, but it must implement `{}`, which is more general", - trait_ref.skip_binder().self_ty(), - implemented_kind, - selected_kind - ) - ); - } - - // Note any argument mismatches - let given_ty = params.skip_binder(); - let expected_ty = trait_ref.skip_binder().substs.type_at(1); - if let ty::Tuple(given) = given_ty.kind() - && let ty::Tuple(expected) = expected_ty.kind() - { - if expected.len() != given.len() { - // Note number of types that were expected and given - err.note( - &format!( - "expected a closure taking {} argument{}, but one taking {} argument{} was given", - given.len(), - pluralize!(given.len()), - expected.len(), - pluralize!(expected.len()), - ) - ); - } else if !self.same_type_modulo_infer(given_ty, expected_ty) { - // Print type mismatch - let (expected_args, given_args) = - self.cmp(given_ty, expected_ty); - err.note_expected_found( - &"a closure with arguments", - expected_args, - &"a closure with arguments", - given_args, - ); - } - } - } else if !trait_ref.has_non_region_infer() - && self.predicate_can_apply(obligation.param_env, trait_predicate) - { - // If a where-clause may be useful, remind the - // user that they can add it. - // - // don't display an on-unimplemented note, as - // these notes will often be of the form - // "the type `T` can't be frobnicated" - // which is somewhat confusing. - self.suggest_restricting_param_bound( - &mut err, - trait_predicate, - None, - obligation.cause.body_id, - ); - } else if !suggested && !unsatisfied_const { - // Can't show anything else useful, try to find similar impls. - let impl_candidates = self.find_similar_impl_candidates(trait_predicate); - if !self.report_similar_impl_candidates( - &impl_candidates, - trait_ref, - body_def_id, - &mut err, - true, - ) { - // This is *almost* equivalent to - // `obligation.cause.code().peel_derives()`, but it gives us the - // trait predicate for that corresponding root obligation. This - // lets us get a derived obligation from a type parameter, like - // when calling `string.strip_suffix(p)` where `p` is *not* an - // implementer of `Pattern<'_>`. - let mut code = obligation.cause.code(); - let mut trait_pred = trait_predicate; - let mut peeled = false; - while let Some((parent_code, parent_trait_pred)) = code.parent() { - code = parent_code; - if let Some(parent_trait_pred) = parent_trait_pred { - trait_pred = parent_trait_pred; - peeled = true; - } - } - let def_id = trait_pred.def_id(); - // Mention *all* the `impl`s for the *top most* obligation, the - // user might have meant to use one of them, if any found. We skip - // auto-traits or fundamental traits that might not be exactly what - // the user might expect to be presented with. Instead this is - // useful for less general traits. - if peeled - && !self.tcx.trait_is_auto(def_id) - && !self.tcx.lang_items().iter().any(|(_, id)| id == def_id) - { - let trait_ref = trait_pred.to_poly_trait_ref(); - let impl_candidates = - self.find_similar_impl_candidates(trait_pred); - self.report_similar_impl_candidates( - &impl_candidates, - trait_ref, - body_def_id, - &mut err, - true, - ); - } - } - - self.maybe_suggest_convert_to_slice( - &mut err, - trait_ref, - impl_candidates.as_slice(), - span, - ); - } + self.try_to_add_help_message( + &obligation, + trait_ref, + &trait_predicate, + &mut err, + span, + is_fn_trait, + suggested, + unsatisfied_const, + ); // Changing mutability doesn't make a difference to whether we have // an `Unsize` impl (Fixes ICE in #71036) @@ -1195,59 +1012,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { ty::PredicateKind::ClosureKind(closure_def_id, closure_substs, kind) => { let found_kind = self.closure_kind(closure_substs).unwrap(); - let closure_span = self.tcx.def_span(closure_def_id); - let mut err = struct_span_err!( - self.tcx.sess, - closure_span, - E0525, - "expected a closure that implements the `{}` trait, \ - but this closure only implements `{}`", - kind, - found_kind - ); - - err.span_label( - closure_span, - format!("this closure implements `{}`, not `{}`", found_kind, kind), - ); - err.span_label( - obligation.cause.span, - format!("the requirement to implement `{}` derives from here", kind), - ); - - // Additional context information explaining why the closure only implements - // a particular trait. - if let Some(typeck_results) = &self.typeck_results { - let hir_id = self - .tcx - .hir() - .local_def_id_to_hir_id(closure_def_id.expect_local()); - match (found_kind, typeck_results.closure_kind_origins().get(hir_id)) { - (ty::ClosureKind::FnOnce, Some((span, place))) => { - err.span_label( - *span, - format!( - "closure is `FnOnce` because it moves the \ - variable `{}` out of its environment", - ty::place_to_string_for_capture(tcx, place) - ), - ); - } - (ty::ClosureKind::FnMut, Some((span, place))) => { - err.span_label( - *span, - format!( - "closure is `FnMut` because it mutates the \ - variable `{}` here", - ty::place_to_string_for_capture(tcx, place) - ), - ); - } - _ => {} - } - } - - err + self.report_closure_error(&obligation, closure_def_id, found_kind, kind) } ty::PredicateKind::WellFormed(ty) => { @@ -1328,117 +1093,21 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { found_trait_ref, expected_trait_ref, terr @ TypeError::CyclicTy(_), - ) => { - let self_ty = found_trait_ref.self_ty().skip_binder(); - let (cause, terr) = if let ty::Closure(def_id, _) = self_ty.kind() { - ( - ObligationCause::dummy_with_span(tcx.def_span(def_id)), - TypeError::CyclicTy(self_ty), - ) - } else { - (obligation.cause.clone(), terr) - }; - self.report_and_explain_type_error( - TypeTrace::poly_trait_refs(&cause, true, expected_trait_ref, found_trait_ref), - terr, - ) - } + ) => self.report_type_parameter_mismatch_cyclic_type_error( + &obligation, + found_trait_ref, + expected_trait_ref, + terr, + ), OutputTypeParameterMismatch(found_trait_ref, expected_trait_ref, _) => { - 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; - } - - let Some(found_trait_ty) = found_trait_ref.self_ty().no_bound_vars() else { - return; - }; - - let found_did = match *found_trait_ty.kind() { - ty::Closure(did, _) - | ty::Foreign(did) - | ty::FnDef(did, _) - | ty::Generator(did, ..) => Some(did), - ty::Adt(def, _) => Some(def.did()), - _ => None, - }; - - let found_node = found_did.and_then(|did| self.tcx.hir().get_if_local(did)); - let found_span = found_did.and_then(|did| self.tcx.hir().span_if_local(did)); - - if self.reported_closure_mismatch.borrow().contains(&(span, found_span)) { - // We check closures twice, with obligations flowing in different directions, - // but we want to complain about them only once. - return; - } - - self.reported_closure_mismatch.borrow_mut().insert((span, found_span)); - - let mut not_tupled = false; - - let found = match found_trait_ref.skip_binder().substs.type_at(1).kind() { - ty::Tuple(ref tys) => vec![ArgKind::empty(); tys.len()], - _ => { - not_tupled = true; - vec![ArgKind::empty()] - } - }; - - let expected_ty = expected_trait_ref.skip_binder().substs.type_at(1); - let expected = match expected_ty.kind() { - ty::Tuple(ref tys) => { - tys.iter().map(|t| ArgKind::from_expected_ty(t, Some(span))).collect() - } - _ => { - not_tupled = true; - vec![ArgKind::Arg("_".to_owned(), expected_ty.to_string())] - } - }; - - // If this is a `Fn` family trait and either the expected or found - // is not tupled, then fall back to just a regular mismatch error. - // This shouldn't be common unless manually implementing one of the - // traits manually, but don't make it more confusing when it does - // happen. - if Some(expected_trait_ref.def_id()) != tcx.lang_items().gen_trait() && not_tupled { - self.report_and_explain_type_error( - TypeTrace::poly_trait_refs( - &obligation.cause, - true, - expected_trait_ref, - found_trait_ref, - ), - ty::error::TypeError::Mismatch, - ) - } else if found.len() == expected.len() { - self.report_closure_arg_mismatch( - span, - found_span, - found_trait_ref, - expected_trait_ref, - obligation.cause.code(), - found_node, - obligation.param_env, - ) - } else { - let (closure_span, closure_arg_span, found) = found_did - .and_then(|did| { - let node = self.tcx.hir().get_if_local(did)?; - let (found_span, closure_arg_span, found) = - self.get_fn_like_arguments(node)?; - Some((Some(found_span), closure_arg_span, found)) - }) - .unwrap_or((found_span, None, found)); - - self.report_arg_count_mismatch( - span, - closure_span, - expected, - found, - found_trait_ty.is_closure(), - closure_arg_span, - ) + match self.report_type_parameter_mismatch_error( + &obligation, + span, + found_trait_ref, + expected_trait_ref, + ) { + Some(err) => err, + None => return, } } @@ -1453,45 +1122,9 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { ) } SelectionError::NotConstEvaluatable(NotConstEvaluatable::MentionsParam) => { - if !self.tcx.features().generic_const_exprs { - let mut err = self.tcx.sess.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 - // (see https://github.com/rust-lang/rust/issues/66962#issuecomment-575907083). - // - // Note that with `feature(generic_const_exprs)` this case should not - // be reachable. - err.note("this may fail depending on what value the parameter takes"); - err.emit(); - return; - } - - match obligation.predicate.kind().skip_binder() { - ty::PredicateKind::ConstEvaluatable(ct) => { - let ty::ConstKind::Unevaluated(uv) = ct.kind() else { - bug!("const evaluatable failed for non-unevaluated const `{ct:?}`"); - }; - let mut err = - self.tcx.sess.struct_span_err(span, "unconstrained generic constant"); - let const_span = self.tcx.def_span(uv.def); - match self.tcx.sess.source_map().span_to_snippet(const_span) { - Ok(snippet) => err.help(&format!( - "try adding a `where` bound using this expression: `where [(); {}]:`", - snippet - )), - _ => err.help("consider adding a `where` bound using this expression"), - }; - err - } - _ => { - span_bug!( - span, - "unexpected non-ConstEvaluatable predicate, this should not be reachable" - ) - } + match self.report_not_const_evaluatable_error(&obligation, span) { + Some(err) => err, + None => return, } } @@ -1563,6 +1196,14 @@ trait InferCtxtPrivExt<'tcx> { other: bool, ) -> bool; + fn report_similar_impl_candidates_for_root_obligation( + &self, + obligation: &PredicateObligation<'tcx>, + trait_predicate: ty::Binder<'tcx, ty::TraitPredicate<'tcx>>, + body_def_id: LocalDefId, + err: &mut Diagnostic, + ); + /// Gets the parent trait chain start fn get_parent_trait_ref( &self, @@ -1626,12 +1267,86 @@ trait InferCtxtPrivExt<'tcx> { cause_code: &ObligationCauseCode<'tcx>, ) -> bool; + fn get_standard_error_message( + &self, + trait_predicate: &ty::PolyTraitPredicate<'tcx>, + message: Option, + predicate_is_const: bool, + append_const_msg: Option>, + post_message: String, + ) -> String; + fn get_safe_transmute_error_and_reason( &self, - obligation: Obligation<'tcx, ty::Predicate<'tcx>>, - trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + obligation: PredicateObligation<'tcx>, + trait_ref: ty::PolyTraitRef<'tcx>, span: Span, ) -> (String, Option); + + fn add_tuple_trait_message( + &self, + obligation_cause_code: &ObligationCauseCode<'tcx>, + err: &mut Diagnostic, + ); + + fn try_to_add_help_message( + &self, + obligation: &PredicateObligation<'tcx>, + trait_ref: ty::PolyTraitRef<'tcx>, + trait_predicate: &ty::PolyTraitPredicate<'tcx>, + err: &mut Diagnostic, + span: Span, + is_fn_trait: bool, + suggested: bool, + unsatisfied_const: bool, + ); + + fn add_help_message_for_fn_trait( + &self, + trait_ref: ty::PolyTraitRef<'tcx>, + err: &mut Diagnostic, + implemented_kind: ty::ClosureKind, + params: ty::Binder<'tcx, Ty<'tcx>>, + ); + + fn maybe_add_note_for_unsatisfied_const( + &self, + obligation: &PredicateObligation<'tcx>, + trait_ref: ty::PolyTraitRef<'tcx>, + trait_predicate: &ty::PolyTraitPredicate<'tcx>, + err: &mut Diagnostic, + span: Span, + ) -> UnsatisfiedConst; + + fn report_closure_error( + &self, + obligation: &PredicateObligation<'tcx>, + closure_def_id: DefId, + found_kind: ty::ClosureKind, + kind: ty::ClosureKind, + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed>; + + fn report_type_parameter_mismatch_cyclic_type_error( + &self, + obligation: &PredicateObligation<'tcx>, + found_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + expected_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + terr: TypeError<'tcx>, + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed>; + + fn report_type_parameter_mismatch_error( + &self, + obligation: &PredicateObligation<'tcx>, + span: Span, + found_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + expected_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + ) -> Option>; + + fn report_not_const_evaluatable_error( + &self, + obligation: &PredicateObligation<'tcx>, + span: Span, + ) -> Option>; } impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { @@ -2195,6 +1910,51 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { report(normalized_impl_candidates, err) } + fn report_similar_impl_candidates_for_root_obligation( + &self, + obligation: &PredicateObligation<'tcx>, + trait_predicate: ty::Binder<'tcx, ty::TraitPredicate<'tcx>>, + body_def_id: LocalDefId, + err: &mut Diagnostic, + ) { + // This is *almost* equivalent to + // `obligation.cause.code().peel_derives()`, but it gives us the + // trait predicate for that corresponding root obligation. This + // lets us get a derived obligation from a type parameter, like + // when calling `string.strip_suffix(p)` where `p` is *not* an + // implementer of `Pattern<'_>`. + let mut code = obligation.cause.code(); + let mut trait_pred = trait_predicate; + let mut peeled = false; + while let Some((parent_code, parent_trait_pred)) = code.parent() { + code = parent_code; + if let Some(parent_trait_pred) = parent_trait_pred { + trait_pred = parent_trait_pred; + peeled = true; + } + } + let def_id = trait_pred.def_id(); + // Mention *all* the `impl`s for the *top most* obligation, the + // user might have meant to use one of them, if any found. We skip + // auto-traits or fundamental traits that might not be exactly what + // the user might expect to be presented with. Instead this is + // useful for less general traits. + if peeled + && !self.tcx.trait_is_auto(def_id) + && !self.tcx.lang_items().iter().any(|(_, id)| id == def_id) + { + let trait_ref = trait_pred.to_poly_trait_ref(); + let impl_candidates = self.find_similar_impl_candidates(trait_pred); + self.report_similar_impl_candidates( + &impl_candidates, + trait_ref, + body_def_id, + err, + true, + ); + } + } + /// Gets the parent trait chain start fn get_parent_trait_ref( &self, @@ -2917,10 +2677,38 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { false } + fn get_standard_error_message( + &self, + trait_predicate: &ty::PolyTraitPredicate<'tcx>, + message: Option, + predicate_is_const: bool, + append_const_msg: Option>, + post_message: String, + ) -> String { + message + .and_then(|cannot_do_this| { + match (predicate_is_const, append_const_msg) { + // do nothing if predicate is not const + (false, _) => Some(cannot_do_this), + // suggested using default post message + (true, Some(None)) => Some(format!("{cannot_do_this} in const contexts")), + // overridden post message + (true, Some(Some(post_message))) => { + Some(format!("{cannot_do_this}{post_message}")) + } + // fallback to generic message + (true, None) => None, + } + }) + .unwrap_or_else(|| { + format!("the trait bound `{}` is not satisfied{}", trait_predicate, post_message,) + }) + } + fn get_safe_transmute_error_and_reason( &self, - obligation: Obligation<'tcx, ty::Predicate<'tcx>>, - trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + obligation: PredicateObligation<'tcx>, + trait_ref: ty::PolyTraitRef<'tcx>, span: Span, ) -> (String, Option) { // Erase regions because layout code doesn't particularly care about regions. @@ -2978,8 +2766,449 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { _ => span_bug!(span, "Unsupported rustc_transmute::Reason variant"), } } + + fn add_tuple_trait_message( + &self, + obligation_cause_code: &ObligationCauseCode<'tcx>, + err: &mut Diagnostic, + ) { + match obligation_cause_code { + ObligationCauseCode::RustCall => { + err.set_primary_message("functions with the \"rust-call\" ABI must take a single non-self tuple argument"); + } + ObligationCauseCode::BindingObligation(def_id, _) + | ObligationCauseCode::ItemObligation(def_id) + if self.tcx.is_fn_trait(*def_id) => + { + err.code(rustc_errors::error_code!(E0059)); + err.set_primary_message(format!( + "type parameter to bare `{}` trait must be a tuple", + self.tcx.def_path_str(*def_id) + )); + } + _ => {} + } + } + + fn try_to_add_help_message( + &self, + obligation: &PredicateObligation<'tcx>, + trait_ref: ty::PolyTraitRef<'tcx>, + trait_predicate: &ty::PolyTraitPredicate<'tcx>, + err: &mut Diagnostic, + span: Span, + is_fn_trait: bool, + suggested: bool, + unsatisfied_const: bool, + ) { + let body_def_id = obligation.cause.body_id; + // Try to report a help message + if is_fn_trait + && let Ok((implemented_kind, params)) = self.type_implements_fn_trait( + obligation.param_env, + trait_ref.self_ty(), + trait_predicate.skip_binder().constness, + trait_predicate.skip_binder().polarity, + ) + { + self.add_help_message_for_fn_trait(trait_ref, err, implemented_kind, params); + } else if !trait_ref.has_non_region_infer() + && self.predicate_can_apply(obligation.param_env, *trait_predicate) + { + // If a where-clause may be useful, remind the + // user that they can add it. + // + // don't display an on-unimplemented note, as + // these notes will often be of the form + // "the type `T` can't be frobnicated" + // which is somewhat confusing. + self.suggest_restricting_param_bound( + err, + *trait_predicate, + None, + obligation.cause.body_id, + ); + } else if !suggested && !unsatisfied_const { + // Can't show anything else useful, try to find similar impls. + let impl_candidates = self.find_similar_impl_candidates(*trait_predicate); + if !self.report_similar_impl_candidates( + &impl_candidates, + trait_ref, + body_def_id, + err, + true, + ) { + self.report_similar_impl_candidates_for_root_obligation(&obligation, *trait_predicate, body_def_id, err); + } + + self.maybe_suggest_convert_to_slice( + err, + trait_ref, + impl_candidates.as_slice(), + span, + ); + } + } + + fn add_help_message_for_fn_trait( + &self, + trait_ref: ty::PolyTraitRef<'tcx>, + err: &mut Diagnostic, + implemented_kind: ty::ClosureKind, + params: ty::Binder<'tcx, Ty<'tcx>>, + ) { + // If the type implements `Fn`, `FnMut`, or `FnOnce`, suppress the following + // suggestion to add trait bounds for the type, since we only typically implement + // these traits once. + + // Note if the `FnMut` or `FnOnce` is less general than the trait we're trying + // to implement. + let selected_kind = self + .tcx + .fn_trait_kind_from_def_id(trait_ref.def_id()) + .expect("expected to map DefId to ClosureKind"); + if !implemented_kind.extends(selected_kind) { + err.note(&format!( + "`{}` implements `{}`, but it must implement `{}`, which is more general", + trait_ref.skip_binder().self_ty(), + implemented_kind, + selected_kind + )); + } + + // Note any argument mismatches + let given_ty = params.skip_binder(); + let expected_ty = trait_ref.skip_binder().substs.type_at(1); + if let ty::Tuple(given) = given_ty.kind() + && let ty::Tuple(expected) = expected_ty.kind() + { + if expected.len() != given.len() { + // Note number of types that were expected and given + err.note( + &format!( + "expected a closure taking {} argument{}, but one taking {} argument{} was given", + given.len(), + pluralize!(given.len()), + expected.len(), + pluralize!(expected.len()), + ) + ); + } else if !self.same_type_modulo_infer(given_ty, expected_ty) { + // Print type mismatch + let (expected_args, given_args) = + self.cmp(given_ty, expected_ty); + err.note_expected_found( + &"a closure with arguments", + expected_args, + &"a closure with arguments", + given_args, + ); + } + } + } + + fn maybe_add_note_for_unsatisfied_const( + &self, + obligation: &PredicateObligation<'tcx>, + trait_ref: ty::PolyTraitRef<'tcx>, + trait_predicate: &ty::PolyTraitPredicate<'tcx>, + err: &mut Diagnostic, + span: Span, + ) -> UnsatisfiedConst { + let mut unsatisfied_const = UnsatisfiedConst(false); + if trait_predicate.is_const_if_const() && obligation.param_env.is_const() { + let non_const_predicate = trait_ref.without_const(); + let non_const_obligation = Obligation { + cause: obligation.cause.clone(), + param_env: obligation.param_env.without_const(), + predicate: non_const_predicate.to_predicate(self.tcx), + recursion_depth: obligation.recursion_depth, + }; + if self.predicate_may_hold(&non_const_obligation) { + unsatisfied_const = UnsatisfiedConst(true); + err.span_note( + span, + &format!( + "the trait `{}` is implemented for `{}`, \ + but that implementation is not `const`", + non_const_predicate.print_modifiers_and_trait_path(), + trait_ref.skip_binder().self_ty(), + ), + ); + } + } + unsatisfied_const + } + + fn report_closure_error( + &self, + obligation: &PredicateObligation<'tcx>, + closure_def_id: DefId, + found_kind: ty::ClosureKind, + kind: ty::ClosureKind, + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { + let closure_span = self.tcx.def_span(closure_def_id); + let mut err = struct_span_err!( + self.tcx.sess, + closure_span, + E0525, + "expected a closure that implements the `{}` trait, \ + but this closure only implements `{}`", + kind, + found_kind + ); + + err.span_label( + closure_span, + format!("this closure implements `{}`, not `{}`", found_kind, kind), + ); + err.span_label( + obligation.cause.span, + format!("the requirement to implement `{}` derives from here", kind), + ); + + // Additional context information explaining why the closure only implements + // a particular trait. + if let Some(typeck_results) = &self.typeck_results { + let hir_id = self.tcx.hir().local_def_id_to_hir_id(closure_def_id.expect_local()); + match (found_kind, typeck_results.closure_kind_origins().get(hir_id)) { + (ty::ClosureKind::FnOnce, Some((span, place))) => { + err.span_label( + *span, + format!( + "closure is `FnOnce` because it moves the \ + variable `{}` out of its environment", + ty::place_to_string_for_capture(self.tcx, place) + ), + ); + } + (ty::ClosureKind::FnMut, Some((span, place))) => { + err.span_label( + *span, + format!( + "closure is `FnMut` because it mutates the \ + variable `{}` here", + ty::place_to_string_for_capture(self.tcx, place) + ), + ); + } + _ => {} + } + } + + err + } + + fn report_type_parameter_mismatch_cyclic_type_error( + &self, + obligation: &PredicateObligation<'tcx>, + found_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + expected_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + terr: TypeError<'tcx>, + ) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> { + let self_ty = found_trait_ref.self_ty().skip_binder(); + let (cause, terr) = if let ty::Closure(def_id, _) = self_ty.kind() { + ( + ObligationCause::dummy_with_span(self.tcx.def_span(def_id)), + TypeError::CyclicTy(self_ty), + ) + } else { + (obligation.cause.clone(), terr) + }; + self.report_and_explain_type_error( + TypeTrace::poly_trait_refs(&cause, true, expected_trait_ref, found_trait_ref), + terr, + ) + } + + fn report_type_parameter_mismatch_error( + &self, + obligation: &PredicateObligation<'tcx>, + span: Span, + found_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + expected_trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + ) -> Option> { + 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; + } + + let Some(found_trait_ty) = found_trait_ref.self_ty().no_bound_vars() else { + return None; + }; + + let found_did = match *found_trait_ty.kind() { + ty::Closure(did, _) | ty::Foreign(did) | ty::FnDef(did, _) | ty::Generator(did, ..) => { + Some(did) + } + ty::Adt(def, _) => Some(def.did()), + _ => None, + }; + + let found_node = found_did.and_then(|did| self.tcx.hir().get_if_local(did)); + let found_span = found_did.and_then(|did| self.tcx.hir().span_if_local(did)); + + if self.reported_closure_mismatch.borrow().contains(&(span, found_span)) { + // We check closures twice, with obligations flowing in different directions, + // but we want to complain about them only once. + return None; + } + + self.reported_closure_mismatch.borrow_mut().insert((span, found_span)); + + let mut not_tupled = false; + + let found = match found_trait_ref.skip_binder().substs.type_at(1).kind() { + ty::Tuple(ref tys) => vec![ArgKind::empty(); tys.len()], + _ => { + not_tupled = true; + vec![ArgKind::empty()] + } + }; + + let expected_ty = expected_trait_ref.skip_binder().substs.type_at(1); + let expected = match expected_ty.kind() { + ty::Tuple(ref tys) => { + tys.iter().map(|t| ArgKind::from_expected_ty(t, Some(span))).collect() + } + _ => { + not_tupled = true; + vec![ArgKind::Arg("_".to_owned(), expected_ty.to_string())] + } + }; + + // If this is a `Fn` family trait and either the expected or found + // is not tupled, then fall back to just a regular mismatch error. + // 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( + if Some(expected_trait_ref.def_id()) != self.tcx.lang_items().gen_trait() && not_tupled + { + self.report_and_explain_type_error( + TypeTrace::poly_trait_refs( + &obligation.cause, + true, + expected_trait_ref, + found_trait_ref, + ), + ty::error::TypeError::Mismatch, + ) + } else if found.len() == expected.len() { + self.report_closure_arg_mismatch( + span, + found_span, + found_trait_ref, + expected_trait_ref, + obligation.cause.code(), + found_node, + obligation.param_env, + ) + } else { + let (closure_span, closure_arg_span, found) = found_did + .and_then(|did| { + let node = self.tcx.hir().get_if_local(did)?; + let (found_span, closure_arg_span, found) = + self.get_fn_like_arguments(node)?; + Some((Some(found_span), closure_arg_span, found)) + }) + .unwrap_or((found_span, None, found)); + + self.report_arg_count_mismatch( + span, + closure_span, + expected, + found, + found_trait_ty.is_closure(), + closure_arg_span, + ) + }, + ) + } + + fn report_not_const_evaluatable_error( + &self, + obligation: &PredicateObligation<'tcx>, + span: Span, + ) -> Option> { + if !self.tcx.features().generic_const_exprs { + let mut err = self + .tcx + .sess + .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 + // (see https://github.com/rust-lang/rust/issues/66962#issuecomment-575907083). + // + // Note that with `feature(generic_const_exprs)` this case should not + // be reachable. + err.note("this may fail depending on what value the parameter takes"); + err.emit(); + return None; + } + + match obligation.predicate.kind().skip_binder() { + ty::PredicateKind::ConstEvaluatable(ct) => { + let ty::ConstKind::Unevaluated(uv) = ct.kind() else { + bug!("const evaluatable failed for non-unevaluated const `{ct:?}`"); + }; + let mut err = self.tcx.sess.struct_span_err(span, "unconstrained generic constant"); + let const_span = self.tcx.def_span(uv.def); + match self.tcx.sess.source_map().span_to_snippet(const_span) { + Ok(snippet) => err.help(&format!( + "try adding a `where` bound using this expression: `where [(); {}]:`", + snippet + )), + _ => err.help("consider adding a `where` bound using this expression"), + }; + Some(err) + } + _ => { + span_bug!( + span, + "unexpected non-ConstEvaluatable predicate, this should not be reachable" + ) + } + } + } } +struct UnsatisfiedConst(pub bool); + +fn get_explanation_based_on_obligation<'tcx>( + obligation: &PredicateObligation<'tcx>, + trait_ref: ty::PolyTraitRef<'tcx>, + trait_predicate: &ty::PolyTraitPredicate<'tcx>, + pre_message: String, +) -> String { + if let ObligationCauseCode::MainFunctionType = obligation.cause.code() { + "consider using `()`, or a `Result`".to_owned() + } else { + let ty_desc = match trait_ref.skip_binder().self_ty().kind() { + ty::FnDef(_, _) => Some("fn item"), + ty::Closure(_, _) => Some("closure"), + _ => None, + }; + + match ty_desc { + Some(desc) => format!( + "{}the trait `{}` is not implemented for {} `{}`", + pre_message, + trait_predicate.print_modifiers_and_trait_path(), + desc, + trait_ref.skip_binder().self_ty(), + ), + None => format!( + "{}the trait `{}` is not implemented for `{}`", + pre_message, + trait_predicate.print_modifiers_and_trait_path(), + trait_ref.skip_binder().self_ty(), + ), + } + } +} /// Crude way of getting back an `Expr` from a `Span`. pub struct FindExprBySpan<'hir> { pub span: Span, diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs index a9c4e126816..193378d7633 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs @@ -341,6 +341,7 @@ pub struct OnUnimplementedNote { /// should fallback to a generic message, `Some(None)` suggests using the default /// appended message, `Some(Some(s))` suggests use the `s` message instead of the /// default one.. + /// FIXME(bryangarza): Change this to an enum with the 3 variants described above. pub append_const_msg: Option>, } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 73207f183a1..c969e5d4975 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -386,7 +386,7 @@ pub trait TypeErrCtxtExt<'tcx> { fn maybe_suggest_convert_to_slice( &self, err: &mut Diagnostic, - trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + trait_ref: ty::PolyTraitRef<'tcx>, candidate_impls: &[ImplCandidate<'tcx>], span: Span, ); @@ -3848,7 +3848,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { fn maybe_suggest_convert_to_slice( &self, err: &mut Diagnostic, - trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>, + trait_ref: ty::PolyTraitRef<'tcx>, candidate_impls: &[ImplCandidate<'tcx>], span: Span, ) { From 55e5a1d20668f4384b647842007095dbf5868851 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Fri, 21 Apr 2023 14:04:22 -0700 Subject: [PATCH 2/9] Create "AppendConstMessage" enum This patch creates an enum to replace a nested `Option`. --- .../src/traits/error_reporting/mod.rs | 19 ++++++------- .../error_reporting/on_unimplemented.rs | 27 ++++++++++++------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index 87b6092bd5d..4cc986774b2 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -15,8 +15,7 @@ use crate::traits::query::evaluate_obligation::InferCtxtExt as _; use crate::traits::query::normalize::QueryNormalizeExt as _; use crate::traits::specialize::to_pretty_impl_header; use crate::traits::NormalizeExt; -use on_unimplemented::OnUnimplementedNote; -use on_unimplemented::TypeErrCtxtExt as _; +use on_unimplemented::{AppendConstMessage, OnUnimplementedNote, TypeErrCtxtExt as _}; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_errors::{ pluralize, struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, @@ -707,7 +706,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { conversion on the error value using the `From` trait" .to_owned(), ), - Some(None), + Some(AppendConstMessage::Default), ) } else { (message, note, append_const_msg) @@ -1272,7 +1271,7 @@ trait InferCtxtPrivExt<'tcx> { trait_predicate: &ty::PolyTraitPredicate<'tcx>, message: Option, predicate_is_const: bool, - append_const_msg: Option>, + append_const_msg: Option, post_message: String, ) -> String; @@ -2682,7 +2681,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { trait_predicate: &ty::PolyTraitPredicate<'tcx>, message: Option, predicate_is_const: bool, - append_const_msg: Option>, + append_const_msg: Option, post_message: String, ) -> String { message @@ -2691,17 +2690,19 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { // do nothing if predicate is not const (false, _) => Some(cannot_do_this), // suggested using default post message - (true, Some(None)) => Some(format!("{cannot_do_this} in const contexts")), + (true, Some(AppendConstMessage::Default)) => { + Some(format!("{cannot_do_this} in const contexts")) + } // overridden post message - (true, Some(Some(post_message))) => { - Some(format!("{cannot_do_this}{post_message}")) + (true, Some(AppendConstMessage::Custom(custom_msg))) => { + Some(format!("{cannot_do_this}{custom_msg}")) } // fallback to generic message (true, None) => None, } }) .unwrap_or_else(|| { - format!("the trait bound `{}` is not satisfied{}", trait_predicate, post_message,) + format!("the trait bound `{}` is not satisfied{}", trait_predicate, post_message) }) } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs index 193378d7633..88525e1b720 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs @@ -327,7 +327,7 @@ pub struct OnUnimplementedDirective { pub label: Option, pub note: Option, pub parent_label: Option, - pub append_const_msg: Option>, + pub append_const_msg: Option, } /// For the `#[rustc_on_unimplemented]` attribute @@ -337,12 +337,21 @@ pub struct OnUnimplementedNote { pub label: Option, pub note: Option, pub parent_label: Option, - /// Append a message for `~const Trait` errors. `None` means not requested and - /// should fallback to a generic message, `Some(None)` suggests using the default - /// appended message, `Some(Some(s))` suggests use the `s` message instead of the - /// default one.. - /// FIXME(bryangarza): Change this to an enum with the 3 variants described above. - pub append_const_msg: Option>, + // If none, should fall back to a generic message + pub append_const_msg: Option, +} + +/// Append a message for `~const Trait` errors. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum AppendConstMessage { + Default, + Custom(Symbol), +} + +impl Default for AppendConstMessage { + fn default() -> Self { + AppendConstMessage::Default + } } impl<'tcx> OnUnimplementedDirective { @@ -420,10 +429,10 @@ impl<'tcx> OnUnimplementedDirective { } } else if item.has_name(sym::append_const_msg) && append_const_msg.is_none() { if let Some(msg) = item.value_str() { - append_const_msg = Some(Some(msg)); + append_const_msg = Some(AppendConstMessage::Custom(msg)); continue; } else if item.is_word() { - append_const_msg = Some(None); + append_const_msg = Some(AppendConstMessage::Default); continue; } } From 047ed32148a39bc38ad18363ded9ab845dd313ee Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Mon, 24 Apr 2023 08:30:05 +0000 Subject: [PATCH 3/9] [LLVM17] Adapt to `ExplicitEmulatedTLS` removal. --- compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp index 90cce9a8650..1acdc95ca8d 100644 --- a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp @@ -410,10 +410,15 @@ extern "C" LLVMTargetMachineRef LLVMRustCreateTargetMachine( } Options.RelaxELFRelocations = RelaxELFRelocations; Options.UseInitArray = UseInitArray; + +#if LLVM_VERSION_LT(17, 0) if (ForceEmulatedTls) { Options.ExplicitEmulatedTLS = true; Options.EmulatedTLS = true; } +#else + Options.EmulatedTLS = ForceEmulatedTls || Trip.hasDefaultEmulatedTLS(); +#endif if (TrapUnreachable) { // Tell LLVM to codegen `unreachable` into an explicit trap instruction. From d4e81bd3fde0165e3e5c5350d76e29339d741076 Mon Sep 17 00:00:00 2001 From: rustbot <47979223+rustbot@users.noreply.github.com> Date: Mon, 24 Apr 2023 13:00:24 -0400 Subject: [PATCH 4/9] Update books --- src/doc/book | 2 +- src/doc/embedded-book | 2 +- src/doc/rustc-dev-guide | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/doc/book b/src/doc/book index c06006157b1..8fa6b854d51 160000 --- a/src/doc/book +++ b/src/doc/book @@ -1 +1 @@ -Subproject commit c06006157b14b3d47b5c716fc392b77f3b2e21ce +Subproject commit 8fa6b854d515506d825390fe0d817f5ef0c89350 diff --git a/src/doc/embedded-book b/src/doc/embedded-book index 701d1551429..897fcf566f1 160000 --- a/src/doc/embedded-book +++ b/src/doc/embedded-book @@ -1 +1 @@ -Subproject commit 701d1551429da4cb609082c0ac99df569e336710 +Subproject commit 897fcf566f16bf87bf37199bdddec1801fd00532 diff --git a/src/doc/rustc-dev-guide b/src/doc/rustc-dev-guide index 6337ed17fb8..2a5eb92197e 160000 --- a/src/doc/rustc-dev-guide +++ b/src/doc/rustc-dev-guide @@ -1 +1 @@ -Subproject commit 6337ed17fb8dcd918d78b7d97d213e923530337c +Subproject commit 2a5eb92197e9cf8fe91164dcbf4f9b88c0d7e73d From d2516c119198f444fe6e4e0dcf95ffea3b8c8930 Mon Sep 17 00:00:00 2001 From: jchzhou Date: Tue, 25 Apr 2023 01:18:20 +0800 Subject: [PATCH 5/9] configure.py: add flag for riscv{64,32}gc musl-root --- src/bootstrap/configure.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/bootstrap/configure.py b/src/bootstrap/configure.py index f95a97518c5..dd1851e29a9 100755 --- a/src/bootstrap/configure.py +++ b/src/bootstrap/configure.py @@ -139,6 +139,10 @@ v("musl-root-mips64", "target.mips64-unknown-linux-muslabi64.musl-root", "mips64-unknown-linux-muslabi64 install directory") v("musl-root-mips64el", "target.mips64el-unknown-linux-muslabi64.musl-root", "mips64el-unknown-linux-muslabi64 install directory") +v("musl-root-riscv32gc", "target.riscv32gc-unknown-linux-musl.musl-root", + "riscv32gc-unknown-linux-musl install directory") +v("musl-root-riscv64gc", "target.riscv64gc-unknown-linux-musl.musl-root", + "riscv64gc-unknown-linux-musl install directory") v("qemu-armhf-rootfs", "target.arm-unknown-linux-gnueabihf.qemu-rootfs", "rootfs in qemu testing, you probably don't want to use this") v("qemu-aarch64-rootfs", "target.aarch64-unknown-linux-gnu.qemu-rootfs", From 653cfdb436256cbc8415630b2537707ae1663806 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Tue, 25 Apr 2023 00:08:29 +0200 Subject: [PATCH 6/9] Revert "Adjust expected result for coverage test" This reverts commit 4da05e0b88d8b51fc6912da2d0b93edb2780e76b. --- .../coverage-reports/expected_show_coverage.issue-84561.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/run-make/coverage-reports/expected_show_coverage.issue-84561.txt b/tests/run-make/coverage-reports/expected_show_coverage.issue-84561.txt index 9c3192c008c..4a60432c14c 100644 --- a/tests/run-make/coverage-reports/expected_show_coverage.issue-84561.txt +++ b/tests/run-make/coverage-reports/expected_show_coverage.issue-84561.txt @@ -136,10 +136,10 @@ 134| | 135| |impl std::fmt::Debug for Foo { 136| | fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - 137| 9| write!(f, "try and succeed")?; + 137| 7| write!(f, "try and succeed")?; ^0 - 138| 9| Ok(()) - 139| 9| } + 138| 7| Ok(()) + 139| 7| } 140| |} 141| | 142| |static mut DEBUG_LEVEL_ENABLED: bool = false; From 33253fa6a42565eeae40e63401a41ac2dcc9e92f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Tue, 25 Apr 2023 00:08:33 +0200 Subject: [PATCH 7/9] Revert "Rename -Zoom=panic to -Zoom=unwind" This reverts commit 4b981c26487ebe56de6b3000fcd98713804beefc. --- compiler/rustc_interface/src/tests.rs | 2 +- compiler/rustc_session/src/config.rs | 6 +++--- compiler/rustc_session/src/options.rs | 2 +- library/alloc/src/alloc.rs | 4 ++-- tests/ui/oom_unwind.rs | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index ce0f90bc1cc..7b0b5102c2d 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -777,7 +777,7 @@ fn test_unstable_options_tracking_hash() { tracked!(no_link, true); tracked!(no_profiler_runtime, true); tracked!(no_unique_section_names, true); - tracked!(oom, OomStrategy::Unwind); + tracked!(oom, OomStrategy::Panic); tracked!(osx_rpath_install_name, true); tracked!(packed_bundled_libs, true); tracked!(panic_abort_tests, true); diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 419b6afe7c6..79eb31bb105 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -3048,9 +3048,9 @@ pub(crate) mod dep_tracking { #[derive(Clone, Copy, PartialEq, Hash, Debug, Encodable, Decodable, HashStable_Generic)] pub enum OomStrategy { /// Generate a panic that can be caught by `catch_unwind`. - Unwind, + Panic, - /// Calls the panic hook as normal but aborts instead of unwinding. + /// Abort the process immediately. Abort, } @@ -3059,7 +3059,7 @@ impl OomStrategy { pub fn should_panic(self) -> u8 { match self { - OomStrategy::Unwind => 1, + OomStrategy::Panic => 1, OomStrategy::Abort => 0, } } diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 98bcacbe7ff..d9f03fe1407 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -662,7 +662,7 @@ mod parse { pub(crate) fn parse_oom_strategy(slot: &mut OomStrategy, v: Option<&str>) -> bool { match v { - Some("unwind") => *slot = OomStrategy::Unwind, + Some("panic") => *slot = OomStrategy::Panic, Some("abort") => *slot = OomStrategy::Abort, _ => return false, } diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index 0db23e55a86..97bedc604da 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -398,7 +398,7 @@ fn rust_oom(layout: Layout) -> ! { fn panic_impl(pi: &core::panic::PanicInfo<'_>) -> !; // This symbol is emitted by rustc . - // Its value depends on the -Zoom={unwind,abort} compiler option. + // Its value depends on the -Zoom={panic,abort} compiler option. static __rust_alloc_error_handler_should_panic: u8; } @@ -458,7 +458,7 @@ pub mod __alloc_error_handler { pub unsafe fn __rdl_oom(size: usize, _align: usize) -> ! { extern "Rust" { // This symbol is emitted by rustc next to __rust_alloc_error_handler. - // Its value depends on the -Zoom={unwind,abort} compiler option. + // Its value depends on the -Zoom={panic,abort} compiler option. static __rust_alloc_error_handler_should_panic: u8; } diff --git a/tests/ui/oom_unwind.rs b/tests/ui/oom_unwind.rs index 704d6f8b810..21a8fb2b22b 100644 --- a/tests/ui/oom_unwind.rs +++ b/tests/ui/oom_unwind.rs @@ -1,4 +1,4 @@ -// compile-flags: -Z oom=unwind +// compile-flags: -Z oom=panic // run-pass // no-prefer-dynamic // needs-unwind From f54dbe6e3116a475f63b580884f07474239a0b25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Tue, 25 Apr 2023 00:08:35 +0200 Subject: [PATCH 8/9] Revert "Remove #[alloc_error_handler] from the compiler and library" This reverts commit abc0660118cc95f47445fd33502a11dd448f5968. --- compiler/rustc_builtin_macros/messages.ftl | 2 + .../src/alloc_error_handler.rs | 97 +++++++++++++++++++ compiler/rustc_builtin_macros/src/errors.rs | 7 ++ compiler/rustc_builtin_macros/src/lib.rs | 2 + .../example/alloc_example.rs | 7 +- .../rustc_codegen_cranelift/src/allocator.rs | 23 ++++- .../example/alloc_example.rs | 7 +- compiler/rustc_codegen_gcc/src/allocator.rs | 34 ++++++- compiler/rustc_codegen_gcc/src/lib.rs | 4 +- compiler/rustc_codegen_llvm/src/allocator.rs | 48 +++++++++ compiler/rustc_codegen_llvm/src/lib.rs | 3 +- .../src/back/symbol_export.rs | 2 +- compiler/rustc_codegen_ssa/src/base.rs | 13 ++- .../rustc_codegen_ssa/src/traits/backend.rs | 1 + compiler/rustc_feature/src/active.rs | 2 + compiler/rustc_feature/src/removed.rs | 2 - compiler/rustc_metadata/messages.ftl | 10 ++ compiler/rustc_metadata/src/creader.rs | 68 +++++++++++++ compiler/rustc_metadata/src/errors.rs | 17 ++++ compiler/rustc_metadata/src/rmeta/decoder.rs | 4 + .../src/rmeta/decoder/cstore_impl.rs | 3 + compiler/rustc_metadata/src/rmeta/encoder.rs | 1 + compiler/rustc_metadata/src/rmeta/mod.rs | 1 + compiler/rustc_middle/src/query/mod.rs | 11 +++ library/alloc/src/alloc.rs | 10 +- library/core/src/macros/mod.rs | 10 ++ library/core/src/prelude/v1.rs | 4 +- library/std/src/alloc.rs | 73 +++++++++++++- library/std/src/lib.rs | 1 + library/std/src/prelude/v1.rs | 4 +- .../crates/hir-def/src/builtin_attr.rs | 4 + tests/run-make/issue-51671/Makefile | 1 + tests/run-make/issue-51671/app.rs | 7 +- tests/run-make/issue-69368/Makefile | 19 ++++ tests/run-make/issue-69368/a.rs | 26 +++++ tests/run-make/issue-69368/b.rs | 8 ++ tests/run-make/issue-69368/c.rs | 34 +++++++ .../run-make/wasm-symbols-not-exported/bar.rs | 7 +- .../alloc-error-handler-bad-signature-1.rs | 18 ++++ ...alloc-error-handler-bad-signature-1.stderr | 44 +++++++++ .../alloc-error-handler-bad-signature-2.rs | 17 ++++ ...alloc-error-handler-bad-signature-2.stderr | 50 ++++++++++ .../alloc-error-handler-bad-signature-3.rs | 15 +++ ...alloc-error-handler-bad-signature-3.stderr | 21 ++++ .../alloc-error/default-alloc-error-hook.rs | 4 +- .../no_std-alloc-error-handler-custom.rs | 84 ++++++++++++++++ .../feature-gate-alloc-error-handler.rs | 16 +++ .../feature-gate-alloc-error-handler.stderr | 12 +++ tests/ui/missing/missing-allocator.rs | 6 ++ 49 files changed, 842 insertions(+), 22 deletions(-) create mode 100644 compiler/rustc_builtin_macros/src/alloc_error_handler.rs create mode 100644 tests/run-make/issue-69368/Makefile create mode 100644 tests/run-make/issue-69368/a.rs create mode 100644 tests/run-make/issue-69368/b.rs create mode 100644 tests/run-make/issue-69368/c.rs create mode 100644 tests/ui/alloc-error/alloc-error-handler-bad-signature-1.rs create mode 100644 tests/ui/alloc-error/alloc-error-handler-bad-signature-1.stderr create mode 100644 tests/ui/alloc-error/alloc-error-handler-bad-signature-2.rs create mode 100644 tests/ui/alloc-error/alloc-error-handler-bad-signature-2.stderr create mode 100644 tests/ui/alloc-error/alloc-error-handler-bad-signature-3.rs create mode 100644 tests/ui/alloc-error/alloc-error-handler-bad-signature-3.stderr create mode 100644 tests/ui/allocator/no_std-alloc-error-handler-custom.rs create mode 100644 tests/ui/feature-gates/feature-gate-alloc-error-handler.rs create mode 100644 tests/ui/feature-gates/feature-gate-alloc-error-handler.stderr diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl index ee2aca6fc93..fca6012a408 100644 --- a/compiler/rustc_builtin_macros/messages.ftl +++ b/compiler/rustc_builtin_macros/messages.ftl @@ -4,6 +4,8 @@ builtin_macros_requires_cfg_pattern = builtin_macros_expected_one_cfg_pattern = expected 1 cfg-pattern +builtin_macros_alloc_error_must_be_fn = alloc_error_handler must be a function + builtin_macros_assert_requires_boolean = macro requires a boolean expression as an argument .label = boolean expression required diff --git a/compiler/rustc_builtin_macros/src/alloc_error_handler.rs b/compiler/rustc_builtin_macros/src/alloc_error_handler.rs new file mode 100644 index 00000000000..82bae9157e7 --- /dev/null +++ b/compiler/rustc_builtin_macros/src/alloc_error_handler.rs @@ -0,0 +1,97 @@ +use crate::errors; +use crate::util::check_builtin_macro_attribute; + +use rustc_ast::ptr::P; +use rustc_ast::{self as ast, FnHeader, FnSig, Generics, StmtKind}; +use rustc_ast::{Fn, ItemKind, Stmt, TyKind, Unsafe}; +use rustc_expand::base::{Annotatable, ExtCtxt}; +use rustc_span::symbol::{kw, sym, Ident}; +use rustc_span::Span; +use thin_vec::{thin_vec, ThinVec}; + +pub fn expand( + ecx: &mut ExtCtxt<'_>, + _span: Span, + meta_item: &ast::MetaItem, + item: Annotatable, +) -> Vec { + check_builtin_macro_attribute(ecx, meta_item, sym::alloc_error_handler); + + let orig_item = item.clone(); + + // Allow using `#[alloc_error_handler]` on an item statement + // FIXME - if we get deref patterns, use them to reduce duplication here + let (item, is_stmt, sig_span) = + if let Annotatable::Item(item) = &item + && let ItemKind::Fn(fn_kind) = &item.kind + { + (item, false, ecx.with_def_site_ctxt(fn_kind.sig.span)) + } else if let Annotatable::Stmt(stmt) = &item + && let StmtKind::Item(item) = &stmt.kind + && let ItemKind::Fn(fn_kind) = &item.kind + { + (item, true, ecx.with_def_site_ctxt(fn_kind.sig.span)) + } else { + ecx.sess.parse_sess.span_diagnostic.emit_err(errors::AllocErrorMustBeFn {span: item.span() }); + return vec![orig_item]; + }; + + // Generate a bunch of new items using the AllocFnFactory + let span = ecx.with_def_site_ctxt(item.span); + + // Generate item statements for the allocator methods. + let stmts = thin_vec![generate_handler(ecx, item.ident, span, sig_span)]; + + // Generate anonymous constant serving as container for the allocator methods. + let const_ty = ecx.ty(sig_span, TyKind::Tup(ThinVec::new())); + let const_body = ecx.expr_block(ecx.block(span, stmts)); + let const_item = ecx.item_const(span, Ident::new(kw::Underscore, span), const_ty, const_body); + let const_item = if is_stmt { + Annotatable::Stmt(P(ecx.stmt_item(span, const_item))) + } else { + Annotatable::Item(const_item) + }; + + // Return the original item and the new methods. + vec![orig_item, const_item] +} + +// #[rustc_std_internal_symbol] +// unsafe fn __rg_oom(size: usize, align: usize) -> ! { +// handler(core::alloc::Layout::from_size_align_unchecked(size, align)) +// } +fn generate_handler(cx: &ExtCtxt<'_>, handler: Ident, span: Span, sig_span: Span) -> Stmt { + let usize = cx.path_ident(span, Ident::new(sym::usize, span)); + let ty_usize = cx.ty_path(usize); + let size = Ident::from_str_and_span("size", span); + let align = Ident::from_str_and_span("align", span); + + let layout_new = cx.std_path(&[sym::alloc, sym::Layout, sym::from_size_align_unchecked]); + let layout_new = cx.expr_path(cx.path(span, layout_new)); + let layout = cx.expr_call( + span, + layout_new, + thin_vec![cx.expr_ident(span, size), cx.expr_ident(span, align)], + ); + + let call = cx.expr_call_ident(sig_span, handler, thin_vec![layout]); + + let never = ast::FnRetTy::Ty(cx.ty(span, TyKind::Never)); + let params = thin_vec![cx.param(span, size, ty_usize.clone()), cx.param(span, align, ty_usize)]; + let decl = cx.fn_decl(params, never); + let header = FnHeader { unsafety: Unsafe::Yes(span), ..FnHeader::default() }; + let sig = FnSig { decl, header, span: span }; + + let body = Some(cx.block_expr(call)); + let kind = ItemKind::Fn(Box::new(Fn { + defaultness: ast::Defaultness::Final, + sig, + generics: Generics::default(), + body, + })); + + let attrs = thin_vec![cx.attr_word(sym::rustc_std_internal_symbol, span)]; + + let item = cx.item(span, Ident::from_str_and_span("__rg_oom", span), attrs, kind); + cx.stmt_item(sig_span, item) +} diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs index bf0ac3f0ee3..630f9b87bc3 100644 --- a/compiler/rustc_builtin_macros/src/errors.rs +++ b/compiler/rustc_builtin_macros/src/errors.rs @@ -19,6 +19,13 @@ pub(crate) struct OneCfgPattern { pub(crate) span: Span, } +#[derive(Diagnostic)] +#[diag(builtin_macros_alloc_error_must_be_fn)] +pub(crate) struct AllocErrorMustBeFn { + #[primary_span] + pub(crate) span: Span, +} + #[derive(Diagnostic)] #[diag(builtin_macros_assert_requires_boolean)] pub(crate) struct AssertRequiresBoolean { diff --git a/compiler/rustc_builtin_macros/src/lib.rs b/compiler/rustc_builtin_macros/src/lib.rs index b6170161d6b..8f86ef44aa3 100644 --- a/compiler/rustc_builtin_macros/src/lib.rs +++ b/compiler/rustc_builtin_macros/src/lib.rs @@ -27,6 +27,7 @@ use rustc_expand::proc_macro::BangProcMacro; use rustc_fluent_macro::fluent_messages; use rustc_span::symbol::sym; +mod alloc_error_handler; mod assert; mod cfg; mod cfg_accessible; @@ -103,6 +104,7 @@ pub fn register_builtin_macros(resolver: &mut dyn ResolverExpand) { } register_attr! { + alloc_error_handler: alloc_error_handler::expand, bench: test::expand_bench, cfg_accessible: cfg_accessible::Expander, cfg_eval: cfg_eval::expand, diff --git a/compiler/rustc_codegen_cranelift/example/alloc_example.rs b/compiler/rustc_codegen_cranelift/example/alloc_example.rs index e39c3272958..4ede2fe4efe 100644 --- a/compiler/rustc_codegen_cranelift/example/alloc_example.rs +++ b/compiler/rustc_codegen_cranelift/example/alloc_example.rs @@ -1,4 +1,4 @@ -#![feature(start, core_intrinsics)] +#![feature(start, core_intrinsics, alloc_error_handler)] #![no_std] extern crate alloc; @@ -22,6 +22,11 @@ fn panic_handler(_: &core::panic::PanicInfo) -> ! { core::intrinsics::abort(); } +#[alloc_error_handler] +fn alloc_error_handler(_: alloc::alloc::Layout) -> ! { + core::intrinsics::abort(); +} + #[start] fn main(_argc: isize, _argv: *const *const u8) -> isize { let world: Box<&str> = Box::new("Hello World!\0"); diff --git a/compiler/rustc_codegen_cranelift/src/allocator.rs b/compiler/rustc_codegen_cranelift/src/allocator.rs index 9fb8079a21f..2c246ceb37d 100644 --- a/compiler/rustc_codegen_cranelift/src/allocator.rs +++ b/compiler/rustc_codegen_cranelift/src/allocator.rs @@ -6,6 +6,7 @@ use crate::prelude::*; use rustc_ast::expand::allocator::{AllocatorKind, AllocatorTy, ALLOCATOR_METHODS}; use rustc_codegen_ssa::base::allocator_kind_for_codegen; use rustc_session::config::OomStrategy; +use rustc_span::symbol::sym; /// Returns whether an allocator shim was created pub(crate) fn codegen( @@ -14,7 +15,13 @@ pub(crate) fn codegen( unwind_context: &mut UnwindContext, ) -> bool { let Some(kind) = allocator_kind_for_codegen(tcx) else { return false }; - codegen_inner(module, unwind_context, kind, tcx.sess.opts.unstable_opts.oom); + codegen_inner( + module, + unwind_context, + kind, + tcx.alloc_error_handler_kind(()).unwrap(), + tcx.sess.opts.unstable_opts.oom, + ); true } @@ -22,6 +29,7 @@ fn codegen_inner( module: &mut impl Module, unwind_context: &mut UnwindContext, kind: AllocatorKind, + alloc_error_handler_kind: AllocatorKind, oom_strategy: OomStrategy, ) { let usize_ty = module.target_config().pointer_type(); @@ -63,6 +71,19 @@ fn codegen_inner( ); } + let sig = Signature { + call_conv: module.target_config().default_call_conv, + params: vec![AbiParam::new(usize_ty), AbiParam::new(usize_ty)], + returns: vec![], + }; + crate::common::create_wrapper_function( + module, + unwind_context, + sig, + "__rust_alloc_error_handler", + &alloc_error_handler_kind.fn_name(sym::oom), + ); + let data_id = module.declare_data(OomStrategy::SYMBOL, Linkage::Export, false, false).unwrap(); let mut data_ctx = DataContext::new(); data_ctx.set_align(1); diff --git a/compiler/rustc_codegen_gcc/example/alloc_example.rs b/compiler/rustc_codegen_gcc/example/alloc_example.rs index faff1dca23f..754e7931412 100644 --- a/compiler/rustc_codegen_gcc/example/alloc_example.rs +++ b/compiler/rustc_codegen_gcc/example/alloc_example.rs @@ -1,4 +1,4 @@ -#![feature(start, core_intrinsics, lang_items)] +#![feature(start, core_intrinsics, alloc_error_handler, lang_items)] #![no_std] extern crate alloc; @@ -21,6 +21,11 @@ fn panic_handler(_: &core::panic::PanicInfo) -> ! { core::intrinsics::abort(); } +#[alloc_error_handler] +fn alloc_error_handler(_: alloc::alloc::Layout) -> ! { + core::intrinsics::abort(); +} + #[lang = "eh_personality"] fn eh_personality() -> ! { loop {} diff --git a/compiler/rustc_codegen_gcc/src/allocator.rs b/compiler/rustc_codegen_gcc/src/allocator.rs index e90db44ece1..4bad33ee879 100644 --- a/compiler/rustc_codegen_gcc/src/allocator.rs +++ b/compiler/rustc_codegen_gcc/src/allocator.rs @@ -5,10 +5,11 @@ use rustc_ast::expand::allocator::{AllocatorKind, AllocatorTy, ALLOCATOR_METHODS use rustc_middle::bug; use rustc_middle::ty::TyCtxt; use rustc_session::config::OomStrategy; +use rustc_span::symbol::sym; use crate::GccContext; -pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, mods: &mut GccContext, _module_name: &str, kind: AllocatorKind) { +pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, mods: &mut GccContext, _module_name: &str, kind: AllocatorKind, alloc_error_handler_kind: AllocatorKind) { let context = &mods.context; let usize = match tcx.sess.target.pointer_width { @@ -86,6 +87,37 @@ pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, mods: &mut GccContext, _module_nam // as described in https://github.com/rust-lang/rust/commit/77a96ed5646f7c3ee8897693decc4626fe380643 } + let types = [usize, usize]; + let name = "__rust_alloc_error_handler".to_string(); + let args: Vec<_> = types.iter().enumerate() + .map(|(index, typ)| context.new_parameter(None, *typ, &format!("param{}", index))) + .collect(); + let func = context.new_function(None, FunctionType::Exported, void, &args, name, false); + + if tcx.sess.target.default_hidden_visibility { + #[cfg(feature="master")] + func.add_attribute(FnAttribute::Visibility(gccjit::Visibility::Hidden)); + } + + let callee = alloc_error_handler_kind.fn_name(sym::oom); + let args: Vec<_> = types.iter().enumerate() + .map(|(index, typ)| context.new_parameter(None, *typ, &format!("param{}", index))) + .collect(); + let callee = context.new_function(None, FunctionType::Extern, void, &args, callee, false); + #[cfg(feature="master")] + callee.add_attribute(FnAttribute::Visibility(gccjit::Visibility::Hidden)); + + let block = func.new_block("entry"); + + let args = args + .iter() + .enumerate() + .map(|(i, _)| func.get_param(i as i32).to_rvalue()) + .collect::>(); + let _ret = context.new_call(None, callee, &args); + //llvm::LLVMSetTailCall(ret, True); + block.end_with_void_return(None); + let name = OomStrategy::SYMBOL.to_string(); let global = context.new_global(None, GlobalKind::Exported, i8, name); let value = tcx.sess.opts.unstable_opts.oom.should_panic(); diff --git a/compiler/rustc_codegen_gcc/src/lib.rs b/compiler/rustc_codegen_gcc/src/lib.rs index 1a20dbcebd4..1cabb05de97 100644 --- a/compiler/rustc_codegen_gcc/src/lib.rs +++ b/compiler/rustc_codegen_gcc/src/lib.rs @@ -163,11 +163,11 @@ impl CodegenBackend for GccCodegenBackend { } impl ExtraBackendMethods for GccCodegenBackend { - fn codegen_allocator<'tcx>(&self, tcx: TyCtxt<'tcx>, module_name: &str, kind: AllocatorKind) -> Self::Module { + fn codegen_allocator<'tcx>(&self, tcx: TyCtxt<'tcx>, module_name: &str, kind: AllocatorKind, alloc_error_handler_kind: AllocatorKind) -> Self::Module { let mut mods = GccContext { context: Context::default(), }; - unsafe { allocator::codegen(tcx, &mut mods, module_name, kind); } + unsafe { allocator::codegen(tcx, &mut mods, module_name, kind, alloc_error_handler_kind); } mods } diff --git a/compiler/rustc_codegen_llvm/src/allocator.rs b/compiler/rustc_codegen_llvm/src/allocator.rs index fc9251dda82..668d9292705 100644 --- a/compiler/rustc_codegen_llvm/src/allocator.rs +++ b/compiler/rustc_codegen_llvm/src/allocator.rs @@ -4,6 +4,7 @@ use rustc_ast::expand::allocator::{AllocatorKind, AllocatorTy, ALLOCATOR_METHODS use rustc_middle::bug; use rustc_middle::ty::TyCtxt; use rustc_session::config::{DebugInfo, OomStrategy}; +use rustc_span::symbol::sym; use crate::debuginfo; use crate::llvm::{self, False, True}; @@ -14,6 +15,7 @@ pub(crate) unsafe fn codegen( module_llvm: &mut ModuleLlvm, module_name: &str, kind: AllocatorKind, + alloc_error_handler_kind: AllocatorKind, ) { let llcx = &*module_llvm.llcx; let llmod = module_llvm.llmod(); @@ -98,6 +100,52 @@ pub(crate) unsafe fn codegen( llvm::LLVMDisposeBuilder(llbuilder); } + // rust alloc error handler + let args = [usize, usize]; // size, align + + let ty = llvm::LLVMFunctionType(void, args.as_ptr(), args.len() as c_uint, False); + let name = "__rust_alloc_error_handler"; + let llfn = llvm::LLVMRustGetOrInsertFunction(llmod, name.as_ptr().cast(), name.len(), ty); + // -> ! DIFlagNoReturn + let no_return = llvm::AttributeKind::NoReturn.create_attr(llcx); + attributes::apply_to_llfn(llfn, llvm::AttributePlace::Function, &[no_return]); + + if tcx.sess.target.default_hidden_visibility { + llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden); + } + if tcx.sess.must_emit_unwind_tables() { + let uwtable = attributes::uwtable_attr(llcx); + attributes::apply_to_llfn(llfn, llvm::AttributePlace::Function, &[uwtable]); + } + + let callee = alloc_error_handler_kind.fn_name(sym::oom); + let callee = llvm::LLVMRustGetOrInsertFunction(llmod, callee.as_ptr().cast(), callee.len(), ty); + // -> ! DIFlagNoReturn + attributes::apply_to_llfn(callee, llvm::AttributePlace::Function, &[no_return]); + llvm::LLVMRustSetVisibility(callee, llvm::Visibility::Hidden); + + let llbb = llvm::LLVMAppendBasicBlockInContext(llcx, llfn, "entry\0".as_ptr().cast()); + + let llbuilder = llvm::LLVMCreateBuilderInContext(llcx); + llvm::LLVMPositionBuilderAtEnd(llbuilder, llbb); + let args = args + .iter() + .enumerate() + .map(|(i, _)| llvm::LLVMGetParam(llfn, i as c_uint)) + .collect::>(); + let ret = llvm::LLVMRustBuildCall( + llbuilder, + ty, + callee, + args.as_ptr(), + args.len() as c_uint, + [].as_ptr(), + 0 as c_uint, + ); + llvm::LLVMSetTailCall(ret, True); + llvm::LLVMBuildRetVoid(llbuilder); + llvm::LLVMDisposeBuilder(llbuilder); + // __rust_alloc_error_handler_should_panic let name = OomStrategy::SYMBOL; let ll_g = llvm::LLVMRustGetOrInsertGlobal(llmod, name.as_ptr().cast(), name.len(), i8); diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 00b2dc1287a..8305a0a4c28 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -115,10 +115,11 @@ impl ExtraBackendMethods for LlvmCodegenBackend { tcx: TyCtxt<'tcx>, module_name: &str, kind: AllocatorKind, + alloc_error_handler_kind: AllocatorKind, ) -> ModuleLlvm { let mut module_llvm = ModuleLlvm::new_metadata(tcx, module_name); unsafe { - allocator::codegen(tcx, &mut module_llvm, module_name, kind); + allocator::codegen(tcx, &mut module_llvm, module_name, kind, alloc_error_handler_kind); } module_llvm } diff --git a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs index bda0cf764e5..8f2f829c17c 100644 --- a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs +++ b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs @@ -219,7 +219,7 @@ fn exported_symbols_provider_local( for symbol_name in ALLOCATOR_METHODS .iter() .map(|method| format!("__rust_{}", method.name)) - .chain([OomStrategy::SYMBOL.to_string()]) + .chain(["__rust_alloc_error_handler".to_string(), OomStrategy::SYMBOL.to_string()]) { let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, &symbol_name)); diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index 3e9d29df02c..c5ca7936a2b 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -635,9 +635,16 @@ pub fn codegen_crate( if let Some(kind) = allocator_kind_for_codegen(tcx) { let llmod_id = cgu_name_builder.build_cgu_name(LOCAL_CRATE, &["crate"], Some("allocator")).to_string(); - let module_llvm = tcx - .sess - .time("write_allocator_module", || backend.codegen_allocator(tcx, &llmod_id, kind)); + let module_llvm = tcx.sess.time("write_allocator_module", || { + backend.codegen_allocator( + tcx, + &llmod_id, + kind, + // If allocator_kind is Some then alloc_error_handler_kind must + // also be Some. + tcx.alloc_error_handler_kind(()).unwrap(), + ) + }); ongoing_codegen.submit_pre_codegened_module_to_llvm( tcx, diff --git a/compiler/rustc_codegen_ssa/src/traits/backend.rs b/compiler/rustc_codegen_ssa/src/traits/backend.rs index 2e88b7ce219..64bebe50ddb 100644 --- a/compiler/rustc_codegen_ssa/src/traits/backend.rs +++ b/compiler/rustc_codegen_ssa/src/traits/backend.rs @@ -123,6 +123,7 @@ pub trait ExtraBackendMethods: CodegenBackend + WriteBackendMethods + Sized + Se tcx: TyCtxt<'tcx>, module_name: &str, kind: AllocatorKind, + alloc_error_handler_kind: AllocatorKind, ) -> Self::Module; /// This generates the codegen unit and returns it along with /// a `u64` giving an estimate of the unit's processing cost. diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 594e6cca912..48f5bd1cb50 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -291,6 +291,8 @@ declare_features! ( (active, abi_x86_interrupt, "1.17.0", Some(40180), None), /// Allows additional const parameter types, such as `&'static str` or user defined types (incomplete, adt_const_params, "1.56.0", Some(95174), None), + /// Allows defining an `#[alloc_error_handler]`. + (active, alloc_error_handler, "1.29.0", Some(51540), None), /// Allows trait methods with arbitrary self types. (active, arbitrary_self_types, "1.23.0", Some(44874), None), /// Allows using `const` operands in inline assembly. diff --git a/compiler/rustc_feature/src/removed.rs b/compiler/rustc_feature/src/removed.rs index c978d6472c8..876a31abdf8 100644 --- a/compiler/rustc_feature/src/removed.rs +++ b/compiler/rustc_feature/src/removed.rs @@ -47,8 +47,6 @@ declare_features! ( (removed, advanced_slice_patterns, "1.0.0", Some(62254), None, Some("merged into `#![feature(slice_patterns)]`")), - /// Allows defining an `#[alloc_error_handler]`. - (removed, alloc_error_handler, "CURRENT_RUSTC_VERSION", Some(51540), None, Some("now handled by panic handler")), (removed, allocator, "1.0.0", None, None, None), /// Allows a test to fail without failing the whole suite. (removed, allow_fail, "1.19.0", Some(46488), None, Some("removed due to no clear use cases")), diff --git a/compiler/rustc_metadata/messages.ftl b/compiler/rustc_metadata/messages.ftl index f0158aeae85..79b8b417257 100644 --- a/compiler/rustc_metadata/messages.ftl +++ b/compiler/rustc_metadata/messages.ftl @@ -155,9 +155,19 @@ metadata_no_multiple_global_alloc = metadata_prev_global_alloc = previous global allocator defined here +metadata_no_multiple_alloc_error_handler = + cannot define multiple allocation error handlers + .label = cannot define a new allocation error handler + +metadata_prev_alloc_error_handler = + previous allocation error handler defined here + metadata_conflicting_global_alloc = the `#[global_allocator]` in {$other_crate_name} conflicts with global allocator in: {$crate_name} +metadata_conflicting_alloc_error_handler = + the `#[alloc_error_handler]` in {$other_crate_name} conflicts with allocation error handler in: {$crate_name} + metadata_global_alloc_required = no global memory allocator found but one is required; link to std or add `#[global_allocator]` to a static item that implements the GlobalAlloc trait diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 8bacd136647..179453238f2 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -38,8 +38,13 @@ pub struct CStore { /// This crate needs an allocator and either provides it itself, or finds it in a dependency. /// If the above is true, then this field denotes the kind of the found allocator. allocator_kind: Option, + /// This crate needs an allocation error handler and either provides it itself, or finds it in a dependency. + /// If the above is true, then this field denotes the kind of the found allocator. + alloc_error_handler_kind: Option, /// This crate has a `#[global_allocator]` item. has_global_allocator: bool, + /// This crate has a `#[alloc_error_handler]` item. + has_alloc_error_handler: bool, /// The interned [StableCrateId]s. pub(crate) stable_crate_ids: StableCrateIdMap, @@ -216,10 +221,18 @@ impl CStore { self.allocator_kind } + pub(crate) fn alloc_error_handler_kind(&self) -> Option { + self.alloc_error_handler_kind + } + pub(crate) fn has_global_allocator(&self) -> bool { self.has_global_allocator } + pub(crate) fn has_alloc_error_handler(&self) -> bool { + self.has_alloc_error_handler + } + pub fn report_unused_deps(&self, tcx: TyCtxt<'_>) { let json_unused_externs = tcx.sess.opts.json_unused_externs; @@ -255,7 +268,9 @@ impl CStore { metas: IndexVec::from_iter(iter::once(None)), injected_panic_runtime: None, allocator_kind: None, + alloc_error_handler_kind: None, has_global_allocator: false, + has_alloc_error_handler: false, stable_crate_ids, unused_externs: Vec::new(), } @@ -761,6 +776,14 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { } spans => !spans.is_empty(), }; + self.cstore.has_alloc_error_handler = match &*alloc_error_handler_spans(krate) { + [span1, span2, ..] => { + self.sess + .emit_err(errors::NoMultipleAllocErrorHandler { span2: *span2, span1: *span1 }); + true + } + spans => !spans.is_empty(), + }; // Check to see if we actually need an allocator. This desire comes // about through the `#![needs_allocator]` attribute and is typically @@ -801,6 +824,21 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { } } } + let mut alloc_error_handler = + self.cstore.has_alloc_error_handler.then(|| Symbol::intern("this crate")); + for (_, data) in self.cstore.iter_crate_data() { + if data.has_alloc_error_handler() { + match alloc_error_handler { + Some(other_crate) => { + self.sess.emit_err(errors::ConflictingAllocErrorHandler { + crate_name: data.name(), + other_crate_name: other_crate, + }); + } + None => alloc_error_handler = Some(data.name()), + } + } + } if global_allocator.is_some() { self.cstore.allocator_kind = Some(AllocatorKind::Global); @@ -816,6 +854,14 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { } self.cstore.allocator_kind = Some(AllocatorKind::Default); } + + if alloc_error_handler.is_some() { + self.cstore.alloc_error_handler_kind = Some(AllocatorKind::Global); + } else { + // The alloc crate provides a default allocation error handler if + // one isn't specified. + self.cstore.alloc_error_handler_kind = Some(AllocatorKind::Default); + } } fn inject_dependency_if( @@ -991,6 +1037,28 @@ fn global_allocator_spans(krate: &ast::Crate) -> Vec { f.spans } +fn alloc_error_handler_spans(krate: &ast::Crate) -> Vec { + struct Finder { + name: Symbol, + spans: Vec, + } + impl<'ast> visit::Visitor<'ast> for Finder { + fn visit_item(&mut self, item: &'ast ast::Item) { + if item.ident.name == self.name + && attr::contains_name(&item.attrs, sym::rustc_std_internal_symbol) + { + self.spans.push(item.span); + } + visit::walk_item(self, item) + } + } + + let name = Symbol::intern(&AllocatorKind::Global.fn_name(sym::oom)); + let mut f = Finder { name, spans: Vec::new() }; + visit::walk_crate(&mut f, krate); + f.spans +} + // On Windows the compiler would sometimes intermittently fail to open the // proc-macro DLL with `Error::LoadLibraryExW`. It is suspected that something in the // system still holds a lock on the file, so we retry a few times before calling it diff --git a/compiler/rustc_metadata/src/errors.rs b/compiler/rustc_metadata/src/errors.rs index 7ecb551a3e5..51b41b5f6a2 100644 --- a/compiler/rustc_metadata/src/errors.rs +++ b/compiler/rustc_metadata/src/errors.rs @@ -352,6 +352,16 @@ pub struct NoMultipleGlobalAlloc { pub span1: Span, } +#[derive(Diagnostic)] +#[diag(metadata_no_multiple_alloc_error_handler)] +pub struct NoMultipleAllocErrorHandler { + #[primary_span] + #[label] + pub span2: Span, + #[label(metadata_prev_alloc_error_handler)] + pub span1: Span, +} + #[derive(Diagnostic)] #[diag(metadata_conflicting_global_alloc)] pub struct ConflictingGlobalAlloc { @@ -359,6 +369,13 @@ pub struct ConflictingGlobalAlloc { pub other_crate_name: Symbol, } +#[derive(Diagnostic)] +#[diag(metadata_conflicting_alloc_error_handler)] +pub struct ConflictingAllocErrorHandler { + pub crate_name: Symbol, + pub other_crate_name: Symbol, +} + #[derive(Diagnostic)] #[diag(metadata_global_alloc_required)] pub struct GlobalAllocRequired; diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 820a502d04c..64245a0460d 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1683,6 +1683,10 @@ impl CrateMetadata { self.root.has_global_allocator } + pub(crate) fn has_alloc_error_handler(&self) -> bool { + self.root.has_alloc_error_handler + } + pub(crate) fn has_default_lib_allocator(&self) -> bool { self.root.has_default_lib_allocator } diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 4aa3768fc3b..141980912b1 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -290,6 +290,7 @@ provide! { tcx, def_id, other, cdata, is_panic_runtime => { cdata.root.panic_runtime } is_compiler_builtins => { cdata.root.compiler_builtins } has_global_allocator => { cdata.root.has_global_allocator } + has_alloc_error_handler => { cdata.root.has_alloc_error_handler } has_panic_handler => { cdata.root.has_panic_handler } is_profiler_runtime => { cdata.root.profiler_runtime } required_panic_strategy => { cdata.root.required_panic_strategy } @@ -378,6 +379,7 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) { // resolve! Does this work? Unsure! That's what the issue is about *providers = Providers { allocator_kind: |tcx, ()| CStore::from_tcx(tcx).allocator_kind(), + alloc_error_handler_kind: |tcx, ()| CStore::from_tcx(tcx).alloc_error_handler_kind(), is_private_dep: |_tcx, LocalCrate| false, native_library: |tcx, id| { tcx.native_libraries(id.krate) @@ -494,6 +496,7 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) { dependency_formats: |tcx, ()| Lrc::new(crate::dependency_format::calculate(tcx)), has_global_allocator: |tcx, LocalCrate| CStore::from_tcx(tcx).has_global_allocator(), + has_alloc_error_handler: |tcx, LocalCrate| CStore::from_tcx(tcx).has_alloc_error_handler(), postorder_cnums: |tcx, ()| { tcx.arena .alloc_slice(&CStore::from_tcx(tcx).crate_dependencies_in_postorder(LOCAL_CRATE)) diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index fd8e49efea0..17a9daee7d0 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -676,6 +676,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { panic_in_drop_strategy: tcx.sess.opts.unstable_opts.panic_in_drop, edition: tcx.sess.edition(), has_global_allocator: tcx.has_global_allocator(LOCAL_CRATE), + has_alloc_error_handler: tcx.has_alloc_error_handler(LOCAL_CRATE), has_panic_handler: tcx.has_panic_handler(LOCAL_CRATE), has_default_lib_allocator: attr::contains_name(&attrs, sym::default_lib_allocator), proc_macro_data, diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 1bb0eabc64f..dd02463e16a 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -225,6 +225,7 @@ pub(crate) struct CrateRoot { panic_in_drop_strategy: PanicStrategy, edition: Edition, has_global_allocator: bool, + has_alloc_error_handler: bool, has_panic_handler: bool, has_default_lib_allocator: bool, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 4b7bc60926e..7109ff95b87 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1351,6 +1351,13 @@ rustc_queries! { desc { "checking if the crate has_global_allocator" } separate_provide_extern } + query has_alloc_error_handler(_: CrateNum) -> bool { + // This query depends on untracked global state in CStore + eval_always + fatal_cycle + desc { "checking if the crate has_alloc_error_handler" } + separate_provide_extern + } query has_panic_handler(_: CrateNum) -> bool { fatal_cycle desc { "checking if the crate has_panic_handler" } @@ -1723,6 +1730,10 @@ rustc_queries! { eval_always desc { "getting the allocator kind for the current crate" } } + query alloc_error_handler_kind(_: ()) -> Option { + eval_always + desc { "alloc error handler kind for the current crate" } + } query upvars_mentioned(def_id: DefId) -> Option<&'tcx FxIndexMap> { desc { |tcx| "collecting upvars mentioned in `{}`", tcx.def_path_str(def_id) } diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index 97bedc604da..08c65a96bc7 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -397,7 +397,7 @@ fn rust_oom(layout: Layout) -> ! { #[lang = "panic_impl"] fn panic_impl(pi: &core::panic::PanicInfo<'_>) -> !; - // This symbol is emitted by rustc . + // This symbol is emitted by rustc next to __rust_alloc_error_handler. // Its value depends on the -Zoom={panic,abort} compiler option. static __rust_alloc_error_handler_should_panic: u8; } @@ -426,6 +426,13 @@ fn rust_oom(layout: Layout) -> ! { /// Callers of memory allocation APIs wishing to abort computation /// in response to an allocation error are encouraged to call this function, /// rather than directly invoking `panic!` or similar. +/// +/// The default behavior of this function is to print a message to standard error +/// and abort the process. +/// It can be replaced with [`set_alloc_error_hook`] and [`take_alloc_error_hook`]. +/// +/// [`set_alloc_error_hook`]: ../../std/alloc/fn.set_alloc_error_hook.html +/// [`take_alloc_error_hook`]: ../../std/alloc/fn.take_alloc_error_hook.html #[stable(feature = "global_alloc", since = "1.28.0")] #[rustc_const_unstable(feature = "const_alloc_error", issue = "92523")] #[cfg(all(not(no_global_oom_handling), not(test)))] @@ -446,7 +453,6 @@ pub const fn handle_alloc_error(layout: Layout) -> ! { #[cfg(all(not(no_global_oom_handling), test))] pub use std::alloc::handle_alloc_error; -#[cfg(bootstrap)] #[cfg(all(not(no_global_oom_handling), not(test)))] #[doc(hidden)] #[allow(unused_attributes)] diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index cc3179ee780..7c93c93b4a0 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -1531,6 +1531,16 @@ pub(crate) mod builtin { /* compiler built-in */ } + /// Attribute macro applied to a function to register it as a handler for allocation failure. + /// + /// See also [`std::alloc::handle_alloc_error`](../../../std/alloc/fn.handle_alloc_error.html). + #[unstable(feature = "alloc_error_handler", issue = "51540")] + #[allow_internal_unstable(rustc_attrs)] + #[rustc_builtin_macro] + pub macro alloc_error_handler($item:item) { + /* compiler built-in */ + } + /// Keeps the item it's applied to if the passed path is accessible, and removes it otherwise. #[unstable( feature = "cfg_accessible", diff --git a/library/core/src/prelude/v1.rs b/library/core/src/prelude/v1.rs index 9c4c0f6ab7a..10525a16f3a 100644 --- a/library/core/src/prelude/v1.rs +++ b/library/core/src/prelude/v1.rs @@ -76,7 +76,9 @@ pub use crate::macros::builtin::{RustcDecodable, RustcEncodable}; // Do not `doc(no_inline)` so that they become doc items on their own // (no public module for them to be re-exported from). #[stable(feature = "builtin_macro_prelude", since = "1.38.0")] -pub use crate::macros::builtin::{bench, derive, global_allocator, test, test_case}; +pub use crate::macros::builtin::{ + alloc_error_handler, bench, derive, global_allocator, test, test_case, +}; #[unstable(feature = "derive_const", issue = "none")] pub use crate::macros::builtin::derive_const; diff --git a/library/std/src/alloc.rs b/library/std/src/alloc.rs index 448a8edc291..c5a5991cc81 100644 --- a/library/std/src/alloc.rs +++ b/library/std/src/alloc.rs @@ -57,8 +57,9 @@ #![stable(feature = "alloc_module", since = "1.28.0")] use core::intrinsics; -use core::ptr; use core::ptr::NonNull; +use core::sync::atomic::{AtomicPtr, Ordering}; +use core::{mem, ptr}; #[stable(feature = "alloc_module", since = "1.28.0")] #[doc(inline)] @@ -285,6 +286,76 @@ unsafe impl Allocator for System { } } +static HOOK: AtomicPtr<()> = AtomicPtr::new(ptr::null_mut()); + +/// Registers a custom allocation error hook, replacing any that was previously registered. +/// +/// The allocation error hook is invoked when an infallible memory allocation fails, before +/// the runtime aborts. The default hook prints a message to standard error, +/// but this behavior can be customized with the [`set_alloc_error_hook`] and +/// [`take_alloc_error_hook`] functions. +/// +/// The hook is provided with a `Layout` struct which contains information +/// about the allocation that failed. +/// +/// The allocation error hook is a global resource. +/// +/// # Examples +/// +/// ``` +/// #![feature(alloc_error_hook)] +/// +/// use std::alloc::{Layout, set_alloc_error_hook}; +/// +/// fn custom_alloc_error_hook(layout: Layout) { +/// panic!("memory allocation of {} bytes failed", layout.size()); +/// } +/// +/// set_alloc_error_hook(custom_alloc_error_hook); +/// ``` +#[unstable(feature = "alloc_error_hook", issue = "51245")] +pub fn set_alloc_error_hook(hook: fn(Layout)) { + HOOK.store(hook as *mut (), Ordering::SeqCst); +} + +/// Unregisters the current allocation error hook, returning it. +/// +/// *See also the function [`set_alloc_error_hook`].* +/// +/// If no custom hook is registered, the default hook will be returned. +#[unstable(feature = "alloc_error_hook", issue = "51245")] +pub fn take_alloc_error_hook() -> fn(Layout) { + let hook = HOOK.swap(ptr::null_mut(), Ordering::SeqCst); + if hook.is_null() { default_alloc_error_hook } else { unsafe { mem::transmute(hook) } } +} + +fn default_alloc_error_hook(layout: Layout) { + extern "Rust" { + // This symbol is emitted by rustc next to __rust_alloc_error_handler. + // Its value depends on the -Zoom={panic,abort} compiler option. + static __rust_alloc_error_handler_should_panic: u8; + } + + #[allow(unused_unsafe)] + if unsafe { __rust_alloc_error_handler_should_panic != 0 } { + panic!("memory allocation of {} bytes failed", layout.size()); + } else { + rtprintpanic!("memory allocation of {} bytes failed\n", layout.size()); + } +} + +#[cfg(not(test))] +#[doc(hidden)] +#[alloc_error_handler] +#[unstable(feature = "alloc_internals", issue = "none")] +pub fn rust_oom(layout: Layout) -> ! { + let hook = HOOK.load(Ordering::SeqCst); + let hook: fn(Layout) = + if hook.is_null() { default_alloc_error_hook } else { unsafe { mem::transmute(hook) } }; + hook(layout); + crate::process::abort() +} + #[cfg(not(test))] #[doc(hidden)] #[allow(unused_attributes)] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 933f75d638b..31400aa1820 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -236,6 +236,7 @@ // // Language features: // tidy-alphabetical-start +#![feature(alloc_error_handler)] #![feature(allocator_internals)] #![feature(allow_internal_unsafe)] #![feature(allow_internal_unstable)] diff --git a/library/std/src/prelude/v1.rs b/library/std/src/prelude/v1.rs index 4f325a70b18..2aefd7c513d 100644 --- a/library/std/src/prelude/v1.rs +++ b/library/std/src/prelude/v1.rs @@ -60,7 +60,9 @@ pub use core::prelude::v1::{RustcDecodable, RustcEncodable}; // Do not `doc(no_inline)` so that they become doc items on their own // (no public module for them to be re-exported from). #[stable(feature = "builtin_macro_prelude", since = "1.38.0")] -pub use core::prelude::v1::{bench, derive, global_allocator, test, test_case}; +pub use core::prelude::v1::{ + alloc_error_handler, bench, derive, global_allocator, test, test_case, +}; #[unstable(feature = "derive_const", issue = "none")] pub use core::prelude::v1::derive_const; diff --git a/src/tools/rust-analyzer/crates/hir-def/src/builtin_attr.rs b/src/tools/rust-analyzer/crates/hir-def/src/builtin_attr.rs index e3e5fac98c0..f7c1e683d0d 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/builtin_attr.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/builtin_attr.rs @@ -381,6 +381,10 @@ pub const INERT_ATTRIBUTES: &[BuiltinAttribute] = &[ rustc_attr!(rustc_allocator, Normal, template!(Word), WarnFollowing, IMPL_DETAIL), rustc_attr!(rustc_nounwind, Normal, template!(Word), WarnFollowing, IMPL_DETAIL), + gated!( + alloc_error_handler, Normal, template!(Word), WarnFollowing, + experimental!(alloc_error_handler) + ), gated!( default_lib_allocator, Normal, template!(Word), WarnFollowing, allocator_internals, experimental!(default_lib_allocator), diff --git a/tests/run-make/issue-51671/Makefile b/tests/run-make/issue-51671/Makefile index 00cf9134662..c9364536992 100644 --- a/tests/run-make/issue-51671/Makefile +++ b/tests/run-make/issue-51671/Makefile @@ -6,3 +6,4 @@ all: $(RUSTC) --emit=obj app.rs nm $(TMPDIR)/app.o | $(CGREP) rust_begin_unwind nm $(TMPDIR)/app.o | $(CGREP) rust_eh_personality + nm $(TMPDIR)/app.o | $(CGREP) __rg_oom diff --git a/tests/run-make/issue-51671/app.rs b/tests/run-make/issue-51671/app.rs index a9d3457bf90..e9dc1e9744f 100644 --- a/tests/run-make/issue-51671/app.rs +++ b/tests/run-make/issue-51671/app.rs @@ -1,5 +1,5 @@ #![crate_type = "bin"] -#![feature(lang_items)] +#![feature(lang_items, alloc_error_handler)] #![no_main] #![no_std] @@ -13,3 +13,8 @@ fn panic(_: &PanicInfo) -> ! { #[lang = "eh_personality"] fn eh() {} + +#[alloc_error_handler] +fn oom(_: Layout) -> ! { + loop {} +} diff --git a/tests/run-make/issue-69368/Makefile b/tests/run-make/issue-69368/Makefile new file mode 100644 index 00000000000..b1229d1b07f --- /dev/null +++ b/tests/run-make/issue-69368/Makefile @@ -0,0 +1,19 @@ +# ignore-cross-compile +include ../tools.mk + +# Test that previously triggered a linker failure with root cause +# similar to one found in the issue #69368. +# +# The crate that provides oom lang item is missing some other lang +# items. Necessary to prevent the use of start-group / end-group. +# +# The weak lang items are defined in a separate compilation units, +# so that linker could omit them if not used. +# +# The crates that need those weak lang items are dependencies of +# crates that provide them. + +all: + $(RUSTC) a.rs + $(RUSTC) b.rs + $(RUSTC) c.rs diff --git a/tests/run-make/issue-69368/a.rs b/tests/run-make/issue-69368/a.rs new file mode 100644 index 00000000000..a54f429550e --- /dev/null +++ b/tests/run-make/issue-69368/a.rs @@ -0,0 +1,26 @@ +#![crate_type = "rlib"] +#![feature(lang_items)] +#![feature(panic_unwind)] +#![no_std] + +extern crate panic_unwind; + +#[panic_handler] +pub fn panic_handler(_: &core::panic::PanicInfo) -> ! { + loop {} +} + +#[no_mangle] +extern "C" fn __rust_drop_panic() -> ! { + loop {} +} + +#[no_mangle] +extern "C" fn __rust_foreign_exception() -> ! { + loop {} +} + +#[lang = "eh_personality"] +fn eh_personality() { + loop {} +} diff --git a/tests/run-make/issue-69368/b.rs b/tests/run-make/issue-69368/b.rs new file mode 100644 index 00000000000..4d6af026656 --- /dev/null +++ b/tests/run-make/issue-69368/b.rs @@ -0,0 +1,8 @@ +#![crate_type = "rlib"] +#![feature(alloc_error_handler)] +#![no_std] + +#[alloc_error_handler] +pub fn error_handler(_: core::alloc::Layout) -> ! { + panic!(); +} diff --git a/tests/run-make/issue-69368/c.rs b/tests/run-make/issue-69368/c.rs new file mode 100644 index 00000000000..729c4249a05 --- /dev/null +++ b/tests/run-make/issue-69368/c.rs @@ -0,0 +1,34 @@ +#![crate_type = "bin"] +#![feature(start)] +#![no_std] + +extern crate alloc; +extern crate a; +extern crate b; + +use alloc::vec::Vec; +use core::alloc::*; + +struct Allocator; + +unsafe impl GlobalAlloc for Allocator { + unsafe fn alloc(&self, _: Layout) -> *mut u8 { + loop {} + } + + unsafe fn dealloc(&self, _: *mut u8, _: Layout) { + loop {} + } +} + +#[global_allocator] +static ALLOCATOR: Allocator = Allocator; + +#[start] +fn main(argc: isize, _argv: *const *const u8) -> isize { + let mut v = Vec::new(); + for i in 0..argc { + v.push(i); + } + v.iter().sum() +} diff --git a/tests/run-make/wasm-symbols-not-exported/bar.rs b/tests/run-make/wasm-symbols-not-exported/bar.rs index eb768446b4b..6ffbd3ec690 100644 --- a/tests/run-make/wasm-symbols-not-exported/bar.rs +++ b/tests/run-make/wasm-symbols-not-exported/bar.rs @@ -1,4 +1,4 @@ -#![feature(panic_handler)] +#![feature(panic_handler, alloc_error_handler)] #![crate_type = "cdylib"] #![no_std] @@ -24,6 +24,11 @@ pub extern fn foo(a: u32) -> u32 { a * 2 } +#[alloc_error_handler] +fn a(_: core::alloc::Layout) -> ! { + loop {} +} + #[panic_handler] fn b(_: &core::panic::PanicInfo) -> ! { loop {} diff --git a/tests/ui/alloc-error/alloc-error-handler-bad-signature-1.rs b/tests/ui/alloc-error/alloc-error-handler-bad-signature-1.rs new file mode 100644 index 00000000000..cd06423e3a5 --- /dev/null +++ b/tests/ui/alloc-error/alloc-error-handler-bad-signature-1.rs @@ -0,0 +1,18 @@ +// compile-flags:-C panic=abort + +#![feature(alloc_error_handler)] +#![no_std] +#![no_main] + +use core::alloc::Layout; + +#[alloc_error_handler] +fn oom( + info: &Layout, //~^ ERROR mismatched types +) -> () //~^^ ERROR mismatched types +{ + loop {} +} + +#[panic_handler] +fn panic(_: &core::panic::PanicInfo) -> ! { loop {} } diff --git a/tests/ui/alloc-error/alloc-error-handler-bad-signature-1.stderr b/tests/ui/alloc-error/alloc-error-handler-bad-signature-1.stderr new file mode 100644 index 00000000000..de92841d7f1 --- /dev/null +++ b/tests/ui/alloc-error/alloc-error-handler-bad-signature-1.stderr @@ -0,0 +1,44 @@ +error[E0308]: mismatched types + --> $DIR/alloc-error-handler-bad-signature-1.rs:10:1 + | +LL | #[alloc_error_handler] + | ---------------------- in this procedural macro expansion +LL | // fn oom( +LL | || info: &Layout, +LL | || ) -> () + | ||_______- arguments to this function are incorrect +LL | | { +LL | | loop {} +LL | | } + | |__^ expected `&Layout`, found `Layout` + | +note: function defined here + --> $DIR/alloc-error-handler-bad-signature-1.rs:10:4 + | +LL | fn oom( + | ^^^ +LL | info: &Layout, + | ------------- + = note: this error originates in the attribute macro `alloc_error_handler` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0308]: mismatched types + --> $DIR/alloc-error-handler-bad-signature-1.rs:10:1 + | +LL | #[alloc_error_handler] + | ---------------------- in this procedural macro expansion +LL | // fn oom( +LL | || info: &Layout, +LL | || ) -> () + | ||_______^ expected `!`, found `()` +LL | | { +LL | | loop {} +LL | | } + | |__- expected `!` because of return type + | + = note: expected type `!` + found unit type `()` + = note: this error originates in the attribute macro `alloc_error_handler` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/alloc-error/alloc-error-handler-bad-signature-2.rs b/tests/ui/alloc-error/alloc-error-handler-bad-signature-2.rs new file mode 100644 index 00000000000..4f76257fc72 --- /dev/null +++ b/tests/ui/alloc-error/alloc-error-handler-bad-signature-2.rs @@ -0,0 +1,17 @@ +// compile-flags:-C panic=abort + +#![feature(alloc_error_handler)] +#![no_std] +#![no_main] + +struct Layout; + +#[alloc_error_handler] +fn oom( + info: Layout, //~^ ERROR mismatched types +) { //~^^ ERROR mismatched types + loop {} +} + +#[panic_handler] +fn panic(_: &core::panic::PanicInfo) -> ! { loop {} } diff --git a/tests/ui/alloc-error/alloc-error-handler-bad-signature-2.stderr b/tests/ui/alloc-error/alloc-error-handler-bad-signature-2.stderr new file mode 100644 index 00000000000..7a495380f2b --- /dev/null +++ b/tests/ui/alloc-error/alloc-error-handler-bad-signature-2.stderr @@ -0,0 +1,50 @@ +error[E0308]: mismatched types + --> $DIR/alloc-error-handler-bad-signature-2.rs:10:1 + | +LL | #[alloc_error_handler] + | ---------------------- in this procedural macro expansion +LL | // fn oom( +LL | || info: Layout, +LL | || ) { + | ||_- arguments to this function are incorrect +LL | | loop {} +LL | | } + | |__^ expected `Layout`, found `core::alloc::Layout` + | + = note: `core::alloc::Layout` and `Layout` have similar names, but are actually distinct types +note: `core::alloc::Layout` is defined in crate `core` + --> $SRC_DIR/core/src/alloc/layout.rs:LL:COL +note: `Layout` is defined in the current crate + --> $DIR/alloc-error-handler-bad-signature-2.rs:7:1 + | +LL | struct Layout; + | ^^^^^^^^^^^^^ +note: function defined here + --> $DIR/alloc-error-handler-bad-signature-2.rs:10:4 + | +LL | fn oom( + | ^^^ +LL | info: Layout, + | ------------ + = note: this error originates in the attribute macro `alloc_error_handler` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0308]: mismatched types + --> $DIR/alloc-error-handler-bad-signature-2.rs:10:1 + | +LL | #[alloc_error_handler] + | ---------------------- in this procedural macro expansion +LL | // fn oom( +LL | || info: Layout, +LL | || ) { + | ||_^ expected `!`, found `()` +LL | | loop {} +LL | | } + | |__- expected `!` because of return type + | + = note: expected type `!` + found unit type `()` + = note: this error originates in the attribute macro `alloc_error_handler` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/alloc-error/alloc-error-handler-bad-signature-3.rs b/tests/ui/alloc-error/alloc-error-handler-bad-signature-3.rs new file mode 100644 index 00000000000..ea9ad39a70d --- /dev/null +++ b/tests/ui/alloc-error/alloc-error-handler-bad-signature-3.rs @@ -0,0 +1,15 @@ +// compile-flags:-C panic=abort + +#![feature(alloc_error_handler)] +#![no_std] +#![no_main] + +struct Layout; + +#[alloc_error_handler] +fn oom() -> ! { //~ ERROR function takes 0 arguments but 1 argument was supplied + loop {} +} + +#[panic_handler] +fn panic(_: &core::panic::PanicInfo) -> ! { loop {} } diff --git a/tests/ui/alloc-error/alloc-error-handler-bad-signature-3.stderr b/tests/ui/alloc-error/alloc-error-handler-bad-signature-3.stderr new file mode 100644 index 00000000000..eb739b149a1 --- /dev/null +++ b/tests/ui/alloc-error/alloc-error-handler-bad-signature-3.stderr @@ -0,0 +1,21 @@ +error[E0061]: this function takes 0 arguments but 1 argument was supplied + --> $DIR/alloc-error-handler-bad-signature-3.rs:10:1 + | +LL | #[alloc_error_handler] + | ---------------------- in this procedural macro expansion +LL | fn oom() -> ! { + | _-^^^^^^^^^^^^ +LL | | loop {} +LL | | } + | |_- unexpected argument of type `core::alloc::Layout` + | +note: function defined here + --> $DIR/alloc-error-handler-bad-signature-3.rs:10:4 + | +LL | fn oom() -> ! { + | ^^^ + = note: this error originates in the attribute macro `alloc_error_handler` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0061`. diff --git a/tests/ui/alloc-error/default-alloc-error-hook.rs b/tests/ui/alloc-error/default-alloc-error-hook.rs index 919d4b714a1..8be09500f4e 100644 --- a/tests/ui/alloc-error/default-alloc-error-hook.rs +++ b/tests/ui/alloc-error/default-alloc-error-hook.rs @@ -2,7 +2,7 @@ // ignore-emscripten no processes // ignore-sgx no processes -use std::alloc::{handle_alloc_error, Layout}; +use std::alloc::{Layout, handle_alloc_error}; use std::env; use std::process::Command; use std::str; @@ -24,5 +24,5 @@ fn main() { .strip_suffix("qemu: uncaught target signal 6 (Aborted) - core dumped\n") .unwrap_or(stderr); - assert!(stderr.contains("memory allocation of 42 bytes failed")); + assert_eq!(stderr, "memory allocation of 42 bytes failed\n"); } diff --git a/tests/ui/allocator/no_std-alloc-error-handler-custom.rs b/tests/ui/allocator/no_std-alloc-error-handler-custom.rs new file mode 100644 index 00000000000..28926243390 --- /dev/null +++ b/tests/ui/allocator/no_std-alloc-error-handler-custom.rs @@ -0,0 +1,84 @@ +// run-pass +// ignore-android no libc +// ignore-emscripten no libc +// ignore-sgx no libc +// ignore-wasm32 no libc +// only-linux +// compile-flags:-C panic=abort +// aux-build:helper.rs + +#![feature(rustc_private, lang_items)] +#![feature(alloc_error_handler)] +#![no_std] +#![no_main] + +extern crate alloc; +extern crate libc; + +// ARM targets need these symbols +#[no_mangle] +pub fn __aeabi_unwind_cpp_pr0() {} + +#[no_mangle] +pub fn __aeabi_unwind_cpp_pr1() {} + +use alloc::boxed::Box; +use alloc::string::ToString; +use core::alloc::{GlobalAlloc, Layout}; +use core::ptr::null_mut; + +extern crate helper; + +struct MyAllocator; + +#[alloc_error_handler] +fn my_oom(layout: Layout) -> ! { + use alloc::fmt::write; + unsafe { + let size = layout.size(); + let mut s = alloc::string::String::new(); + write(&mut s, format_args!("My OOM: failed to allocate {} bytes!\n", size)).unwrap(); + libc::write(libc::STDERR_FILENO, s.as_ptr() as *const _, s.len()); + libc::exit(0) + } +} + +unsafe impl GlobalAlloc for MyAllocator { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + if layout.size() < 4096 { libc::malloc(layout.size()) as _ } else { null_mut() } + } + unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {} +} + +#[global_allocator] +static A: MyAllocator = MyAllocator; + +#[panic_handler] +fn panic(panic_info: &core::panic::PanicInfo) -> ! { + unsafe { + let s = panic_info.to_string(); + const PSTR: &str = "panic occurred: "; + const CR: &str = "\n"; + libc::write(libc::STDERR_FILENO, PSTR.as_ptr() as *const _, PSTR.len()); + libc::write(libc::STDERR_FILENO, s.as_ptr() as *const _, s.len()); + libc::write(libc::STDERR_FILENO, CR.as_ptr() as *const _, CR.len()); + libc::exit(1) + } +} + +// Because we are compiling this code with `-C panic=abort`, this wouldn't normally be needed. +// However, `core` and `alloc` are both compiled with `-C panic=unwind`, which means that functions +// in these libraries will refer to `rust_eh_personality` if LLVM can not *prove* the contents won't +// unwind. So, for this test case we will define the symbol. +#[lang = "eh_personality"] +extern "C" fn rust_eh_personality() {} + +#[derive(Default, Debug)] +struct Page(#[allow(unused_tuple_struct_fields)] [[u64; 32]; 16]); + +#[no_mangle] +fn main(_argc: i32, _argv: *const *const u8) -> isize { + let zero = Box::::new(Default::default()); + helper::work_with(&zero); + 1 +} diff --git a/tests/ui/feature-gates/feature-gate-alloc-error-handler.rs b/tests/ui/feature-gates/feature-gate-alloc-error-handler.rs new file mode 100644 index 00000000000..78d189d20b6 --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-alloc-error-handler.rs @@ -0,0 +1,16 @@ +// compile-flags:-C panic=abort + +#![no_std] +#![no_main] + +use core::alloc::Layout; + +#[alloc_error_handler] //~ ERROR use of unstable library feature 'alloc_error_handler' +fn oom(info: Layout) -> ! { + loop {} +} + +#[panic_handler] +fn panic(_: &core::panic::PanicInfo) -> ! { + loop {} +} diff --git a/tests/ui/feature-gates/feature-gate-alloc-error-handler.stderr b/tests/ui/feature-gates/feature-gate-alloc-error-handler.stderr new file mode 100644 index 00000000000..f414eb463df --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-alloc-error-handler.stderr @@ -0,0 +1,12 @@ +error[E0658]: use of unstable library feature 'alloc_error_handler' + --> $DIR/feature-gate-alloc-error-handler.rs:8:3 + | +LL | #[alloc_error_handler] + | ^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #51540 for more information + = help: add `#![feature(alloc_error_handler)]` to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. diff --git a/tests/ui/missing/missing-allocator.rs b/tests/ui/missing/missing-allocator.rs index e06e603e3bf..2dc509f2c63 100644 --- a/tests/ui/missing/missing-allocator.rs +++ b/tests/ui/missing/missing-allocator.rs @@ -3,10 +3,16 @@ #![no_std] #![crate_type = "staticlib"] +#![feature(alloc_error_handler)] #[panic_handler] fn panic(_: &core::panic::PanicInfo) -> ! { loop {} } +#[alloc_error_handler] +fn oom(_: core::alloc::Layout) -> ! { + loop {} +} + extern crate alloc; From 23a363821de3276747b27754bd0dd03a32991187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Tue, 25 Apr 2023 00:08:37 +0200 Subject: [PATCH 9/9] Revert "Report allocation errors as panics" This reverts commit c9a6e41026d7aa27d897fb83e995447719753076. --- library/alloc/Cargo.toml | 3 -- library/alloc/src/alloc.rs | 84 ++++-------------------------------- library/alloc/src/lib.rs | 1 - library/std/Cargo.toml | 2 +- library/std/src/lib.rs | 1 - library/std/src/panicking.rs | 43 ++++++++---------- 6 files changed, 27 insertions(+), 107 deletions(-) diff --git a/library/alloc/Cargo.toml b/library/alloc/Cargo.toml index 8975ba3f06b..95c07abf731 100644 --- a/library/alloc/Cargo.toml +++ b/library/alloc/Cargo.toml @@ -35,6 +35,3 @@ compiler-builtins-mem = ['compiler_builtins/mem'] compiler-builtins-c = ["compiler_builtins/c"] compiler-builtins-no-asm = ["compiler_builtins/no-asm"] compiler-builtins-mangled-names = ["compiler_builtins/mangled-names"] - -# Make panics and failed asserts immediately abort without formatting any message -panic_immediate_abort = ["core/panic_immediate_abort"] diff --git a/library/alloc/src/alloc.rs b/library/alloc/src/alloc.rs index 08c65a96bc7..6f2ba957bcd 100644 --- a/library/alloc/src/alloc.rs +++ b/library/alloc/src/alloc.rs @@ -14,11 +14,6 @@ use core::ptr::{self, NonNull}; #[doc(inline)] pub use core::alloc::*; -#[cfg(not(no_global_oom_handling))] -use core::any::Any; -#[cfg(not(no_global_oom_handling))] -use core::panic::BoxMeUp; - #[cfg(test)] mod tests; @@ -348,77 +343,14 @@ pub(crate) unsafe fn box_free(ptr: Unique, alloc: A) } } -/// Payload passed to the panic handler when `handle_alloc_error` is called. -#[unstable(feature = "panic_oom_payload", issue = "none")] -#[derive(Debug)] -pub struct AllocErrorPanicPayload { - layout: Layout, -} - -impl AllocErrorPanicPayload { - /// Internal function for the standard library to clone a payload. - #[unstable(feature = "std_internals", issue = "none")] - #[doc(hidden)] - pub fn internal_clone(&self) -> Self { - AllocErrorPanicPayload { layout: self.layout } - } - - /// Returns the [`Layout`] of the allocation attempt that caused the error. - #[unstable(feature = "panic_oom_payload", issue = "none")] - pub fn layout(&self) -> Layout { - self.layout - } -} - -#[unstable(feature = "std_internals", issue = "none")] -#[cfg(not(no_global_oom_handling))] -unsafe impl BoxMeUp for AllocErrorPanicPayload { - fn take_box(&mut self) -> *mut (dyn Any + Send) { - use crate::boxed::Box; - Box::into_raw(Box::new(self.internal_clone())) - } - - fn get(&mut self) -> &(dyn Any + Send) { - self - } -} - // # Allocation error handler -#[cfg(all(not(no_global_oom_handling), not(test)))] -fn rust_oom(layout: Layout) -> ! { - if cfg!(feature = "panic_immediate_abort") { - core::intrinsics::abort() - } - - extern "Rust" { - // NOTE This function never crosses the FFI boundary; it's a Rust-to-Rust call - // that gets resolved to the `#[panic_handler]` function. - #[lang = "panic_impl"] - fn panic_impl(pi: &core::panic::PanicInfo<'_>) -> !; - - // This symbol is emitted by rustc next to __rust_alloc_error_handler. - // Its value depends on the -Zoom={panic,abort} compiler option. - static __rust_alloc_error_handler_should_panic: u8; - } - - // Hack to work around issues with the lifetime of Arguments. - match format_args!("memory allocation of {} bytes failed", layout.size()) { - fmt => { - // Create a PanicInfo with a custom payload for the panic handler. - let can_unwind = unsafe { __rust_alloc_error_handler_should_panic != 0 }; - let mut pi = core::panic::PanicInfo::internal_constructor( - Some(&fmt), - core::panic::Location::caller(), - can_unwind, - ); - let payload = AllocErrorPanicPayload { layout }; - pi.set_payload(&payload); - - // SAFETY: `panic_impl` is defined in safe Rust code and thus is safe to call. - unsafe { panic_impl(&pi) } - } - } +#[cfg(not(no_global_oom_handling))] +extern "Rust" { + // This is the magic symbol to call the global alloc error handler. rustc generates + // it to call `__rg_oom` if there is a `#[alloc_error_handler]`, or to call the + // default implementations below (`__rdl_oom`) otherwise. + fn __rust_alloc_error_handler(size: usize, align: usize) -> !; } /// Abort on memory allocation error or failure. @@ -443,7 +375,9 @@ pub const fn handle_alloc_error(layout: Layout) -> ! { } fn rt_error(layout: Layout) -> ! { - rust_oom(layout); + unsafe { + __rust_alloc_error_handler(layout.size(), layout.align()); + } } unsafe { core::intrinsics::const_eval_select((layout,), ct_error, rt_error) } diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 2c6a266e2a1..a002421aeef 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -135,7 +135,6 @@ #![feature(maybe_uninit_slice)] #![feature(maybe_uninit_uninit_array)] #![feature(maybe_uninit_uninit_array_transpose)] -#![feature(panic_internals)] #![feature(pattern)] #![feature(pointer_byte_offsets)] #![feature(provide_any)] diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index 109387b09d0..f2fda64a1ee 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -67,7 +67,7 @@ llvm-libunwind = ["unwind/llvm-libunwind"] system-llvm-libunwind = ["unwind/system-llvm-libunwind"] # Make panics and failed asserts immediately abort without formatting any message -panic_immediate_abort = ["alloc/panic_immediate_abort"] +panic_immediate_abort = ["core/panic_immediate_abort"] # Enable std_detect default features for stdarch/crates/std_detect: # https://github.com/rust-lang/stdarch/blob/master/crates/std_detect/Cargo.toml diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 31400aa1820..318a46d1b63 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -320,7 +320,6 @@ #![feature(get_mut_unchecked)] #![feature(map_try_insert)] #![feature(new_uninit)] -#![feature(panic_oom_payload)] #![feature(slice_concat_trait)] #![feature(thin_box)] #![feature(try_reserve_kind)] diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index ca4cf68ad54..a46a29cbad6 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -245,24 +245,19 @@ fn default_hook(info: &PanicInfo<'_>) { // The current implementation always returns `Some`. let location = info.location().unwrap(); + + let msg = match info.payload().downcast_ref::<&'static str>() { + Some(s) => *s, + None => match info.payload().downcast_ref::() { + Some(s) => &s[..], + None => "Box", + }, + }; let thread = thread_info::current_thread(); let name = thread.as_ref().and_then(|t| t.name()).unwrap_or(""); let write = |err: &mut dyn crate::io::Write| { - // Use the panic message directly if available, otherwise take it from - // the payload. - if let Some(msg) = info.message() { - let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}"); - } else { - let msg = if let Some(s) = info.payload().downcast_ref::<&'static str>() { - *s - } else if let Some(s) = info.payload().downcast_ref::() { - &s[..] - } else { - "Box" - }; - let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}"); - } + let _ = writeln!(err, "thread '{name}' panicked at '{msg}', {location}"); static FIRST_PANIC: AtomicBool = AtomicBool::new(true); @@ -529,8 +524,6 @@ pub fn panicking() -> bool { #[cfg(not(test))] #[panic_handler] pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { - use alloc::alloc::AllocErrorPanicPayload; - struct PanicPayload<'a> { inner: &'a fmt::Arguments<'a>, string: Option, @@ -557,7 +550,8 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { unsafe impl<'a> BoxMeUp for PanicPayload<'a> { fn take_box(&mut self) -> *mut (dyn Any + Send) { // We do two allocations here, unfortunately. But (a) they're required with the current - // scheme, and (b) OOM uses its own separate payload type which doesn't allocate. + // scheme, and (b) we don't handle panic + OOM properly anyway (see comment in + // begin_panic below). let contents = mem::take(self.fill()); Box::into_raw(Box::new(contents)) } @@ -582,14 +576,7 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { let loc = info.location().unwrap(); // The current implementation always returns Some let msg = info.message().unwrap(); // The current implementation always returns Some crate::sys_common::backtrace::__rust_end_short_backtrace(move || { - if let Some(payload) = info.payload().downcast_ref::() { - rust_panic_with_hook( - &mut payload.internal_clone(), - info.message(), - loc, - info.can_unwind(), - ); - } else if let Some(msg) = msg.as_str() { + if let Some(msg) = msg.as_str() { rust_panic_with_hook(&mut StrPanicPayload(msg), info.message(), loc, info.can_unwind()); } else { rust_panic_with_hook( @@ -636,7 +623,11 @@ pub const fn begin_panic(msg: M) -> ! { unsafe impl BoxMeUp for PanicPayload { fn take_box(&mut self) -> *mut (dyn Any + Send) { - // Note that this should be the only allocation performed in this code path. + // Note that this should be the only allocation performed in this code path. Currently + // this means that panic!() on OOM will invoke this code path, but then again we're not + // really ready for panic on OOM anyway. If we do start doing this, then we should + // propagate this allocation to be performed in the parent of this thread instead of the + // thread that's panicking. let data = match self.inner.take() { Some(a) => Box::new(a) as Box, None => process::abort(),