-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
rustdoc: fix ICE when delegating functions with impl Trait as argument type
#156498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -858,6 +858,67 @@ pub(crate) fn clean_generics<'tcx>( | |
| } | ||
| } | ||
|
|
||
| /// Cleans delegation generics with HIR ordering and ty-side fallback. | ||
| fn clean_delegation_generics<'tcx>( | ||
| gens: &hir::Generics<'tcx>, | ||
| cx: &mut DocContext<'tcx>, | ||
| ty_generics: Generics, | ||
| ) -> Generics { | ||
| let mut hir_generics = clean_generics(gens, cx); | ||
| let impl_trait_param_def_ids = hir_generics | ||
| .params | ||
| .iter() | ||
| .filter(|param| param.is_synthetic_param()) | ||
| .map(|param| param.def_id) | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| for def_id in impl_trait_param_def_ids { | ||
| cx.impl_trait_bounds.remove(&def_id.into()); | ||
| } | ||
| hir_generics.params.retain(|param| !param.is_synthetic_param()); | ||
| hir_generics.where_predicates = | ||
| hir_generics | ||
| .where_predicates | ||
| .into_iter() | ||
| .filter_map(|mut pred| { | ||
| if retain_delegation_predicate(cx.tcx, &mut pred) { Some(pred) } else { None } | ||
| }) | ||
| .collect(); | ||
|
|
||
| let Generics { params: ty_params, where_predicates } = ty_generics; | ||
| let where_predicates = | ||
| where_predicates | ||
| .into_iter() | ||
| .filter_map(|mut pred| { | ||
| if retain_delegation_predicate(cx.tcx, &mut pred) { Some(pred) } else { None } | ||
| }) | ||
| .collect(); | ||
| let mut generics = Generics { params: hir_generics.params, where_predicates }; | ||
|
|
||
| for param in ty_params { | ||
| if !generics.params.iter().any(|existing| existing.def_id == param.def_id) { | ||
| generics.params.push(param); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still seems like it has the potential to reorder generic parameters. ideally we could do this in one map pipeline. Is there something here that would make such an approach infeasible, such as Here's the type of solution I had in mind: let generic_params = hir_generics.params
.iter()
.zip(ty_params.iter())
.map(|(hir_param, ty_param)| if hir_param.is_synthetic_param() { ty_param } else { hir_param }); |
||
| } | ||
| for pred in hir_generics.where_predicates { | ||
| if !generics.where_predicates.contains(&pred) { | ||
| generics.where_predicates.push(pred); | ||
| } | ||
| } | ||
|
|
||
| generics | ||
| } | ||
|
|
||
| fn retain_delegation_predicate(tcx: TyCtxt<'_>, pred: &mut WherePredicate) -> bool { | ||
| match pred { | ||
| WherePredicate::BoundPredicate { bounds, .. } => { | ||
| bounds.retain(|bound| !bound.is_meta_sized_bound(tcx)); | ||
| !bounds.is_empty() | ||
| } | ||
| WherePredicate::RegionPredicate { .. } | WherePredicate::EqPredicate { .. } => true, | ||
| } | ||
| } | ||
|
|
||
| fn clean_ty_generics<'tcx>(cx: &mut DocContext<'tcx>, def_id: DefId) -> Generics { | ||
| clean_ty_generics_inner(cx, cx.tcx.generics_of(def_id), cx.tcx.explicit_predicates_of(def_id)) | ||
| } | ||
|
|
@@ -1091,10 +1152,10 @@ fn clean_fn_or_proc_macro<'tcx>( | |
| None => { | ||
| let mut func = clean_function( | ||
| cx, | ||
| item.owner_id.to_def_id(), | ||
| sig, | ||
| generics, | ||
| ParamsSrc::Body(body_id), | ||
| item.owner_id.to_def_id(), | ||
| ); | ||
| clean_fn_decl_legacy_const_generics(&mut func, attrs); | ||
| FunctionItem(func) | ||
|
|
@@ -1130,33 +1191,32 @@ enum ParamsSrc<'tcx> { | |
|
|
||
| fn clean_function<'tcx>( | ||
| cx: &mut DocContext<'tcx>, | ||
| def_id: DefId, | ||
| sig: &hir::FnSig<'tcx>, | ||
| generics: &hir::Generics<'tcx>, | ||
| params: ParamsSrc<'tcx>, | ||
| def_id: DefId, | ||
| ) -> Box<Function> { | ||
| if sig.decl.opt_delegation_sig_id().is_some() { | ||
| return enter_impl_trait(cx, |cx| { | ||
| // Delegation HIR signatures are unresolved, so clean the resolved ty-side function. | ||
| let mut function = inline::build_function(cx, def_id); | ||
| let ty_generics = mem::take(&mut function.generics); | ||
| function.generics = clean_delegation_generics(generics, cx, ty_generics); | ||
| function | ||
| }); | ||
| } | ||
|
|
||
| let (generics, decl) = enter_impl_trait(cx, |cx| { | ||
| // NOTE: Generics must be cleaned before params. | ||
| let generics = clean_generics(generics, cx); | ||
| let decl = if sig.decl.opt_delegation_sig_id().is_some() { | ||
| // A delegation item (`reuse path::method`) has no resolved signature in the | ||
| // HIR: its inputs and return type are `InferDelegation` nodes that clean to | ||
| // `_`, and an `async` header over that inferred return type would panic in | ||
| // `sugared_async_return_type`. The resolved signature only exists on the ty | ||
| // side, so clean that instead, exactly like an inlined item. This both fixes | ||
| // the rendered `-> _` / `self: _` and makes the async sugaring well-defined. | ||
| let sig = cx.tcx.fn_sig(def_id).instantiate_identity().skip_norm_wip(); | ||
| clean_poly_fn_sig(cx, Some(def_id), sig) | ||
| } else { | ||
| let params = match params { | ||
| ParamsSrc::Body(body_id) => clean_params_via_body(cx, sig.decl.inputs, body_id), | ||
| // Let's not perpetuate anon params from Rust 2015; use `_` for them. | ||
| ParamsSrc::Idents(idents) => clean_params(cx, sig.decl.inputs, idents, |ident| { | ||
| Some(ident.map_or(kw::Underscore, |ident| ident.name)) | ||
| }), | ||
| }; | ||
| clean_fn_decl_with_params(cx, sig.decl, Some(&sig.header), params) | ||
| let params = match params { | ||
| ParamsSrc::Body(body_id) => clean_params_via_body(cx, sig.decl.inputs, body_id), | ||
| // Let's not perpetuate anon params from Rust 2015; use `_` for them. | ||
| ParamsSrc::Idents(idents) => clean_params(cx, sig.decl.inputs, idents, |ident| { | ||
| Some(ident.map_or(kw::Underscore, |ident| ident.name)) | ||
| }), | ||
| }; | ||
| let decl = clean_fn_decl_with_params(cx, sig.decl, Some(&sig.header), params); | ||
| (generics, decl) | ||
| }); | ||
| Box::new(Function { decl, generics }) | ||
|
|
@@ -1289,16 +1349,16 @@ fn clean_trait_item<'tcx>(trait_item: &hir::TraitItem<'tcx>, cx: &mut DocContext | |
| } | ||
| hir::TraitItemKind::Fn(ref sig, hir::TraitFn::Provided(body)) => { | ||
| let m = | ||
| clean_function(cx, sig, trait_item.generics, ParamsSrc::Body(body), local_did); | ||
| clean_function(cx, local_did, sig, trait_item.generics, ParamsSrc::Body(body)); | ||
| MethodItem(m, Defaultness::from_trait_item(trait_item.defaultness)) | ||
| } | ||
| hir::TraitItemKind::Fn(ref sig, hir::TraitFn::Required(idents)) => { | ||
| let m = clean_function( | ||
| cx, | ||
| local_did, | ||
| sig, | ||
| trait_item.generics, | ||
| ParamsSrc::Idents(idents), | ||
| local_did, | ||
| ); | ||
| RequiredMethodItem(m, Defaultness::from_trait_item(trait_item.defaultness)) | ||
| } | ||
|
|
@@ -1340,7 +1400,7 @@ pub(crate) fn clean_impl_item<'tcx>( | |
| type_: clean_ty(ty, cx), | ||
| })), | ||
| hir::ImplItemKind::Fn(ref sig, body) => { | ||
| let m = clean_function(cx, sig, impl_.generics, ParamsSrc::Body(body), local_did); | ||
| let m = clean_function(cx, local_did, sig, impl_.generics, ParamsSrc::Body(body)); | ||
| let defaultness = match impl_.impl_kind { | ||
| hir::ImplItemImplKind::Inherent { .. } => hir::Defaultness::Final, | ||
| hir::ImplItemImplKind::Trait { defaultness, .. } => defaultness, | ||
|
|
@@ -3279,7 +3339,7 @@ fn clean_maybe_renamed_foreign_item<'tcx>( | |
| cx.with_param_env(def_id, |cx| { | ||
| let kind = match item.kind { | ||
| hir::ForeignItemKind::Fn(sig, idents, generics) => ForeignFunctionItem( | ||
| clean_function(cx, &sig, generics, ParamsSrc::Idents(idents), def_id), | ||
| clean_function(cx, def_id, &sig, generics, ParamsSrc::Idents(idents)), | ||
| sig.header.safety(), | ||
| ), | ||
| hir::ForeignItemKind::Static(ty, mutability, safety) => ForeignStaticItem( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| #![feature(fn_delegation)] | ||
| #![allow(incomplete_features)] | ||
|
|
||
| pub fn external(_: impl FnOnce()) {} | ||
|
|
||
| fn delegated_to(_: impl FnOnce()) {} | ||
|
|
||
| pub reuse delegated_to as delegated; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // Regression test for <https://github.com/rust-lang/rust/issues/155728> | ||
| // Make sure delegating functions with `impl Trait` in argument position works correctly. | ||
| //@ aux-crate:aux=fn-delegation-impl-trait-aux.rs | ||
| //@ edition: 2021 | ||
| #![feature(fn_delegation)] | ||
| #![allow(incomplete_features)] | ||
|
|
||
|
cijiugechu marked this conversation as resolved.
|
||
| fn foo(_: impl FnOnce()) {} | ||
|
|
||
| //@ has fn_delegation_impl_trait/fn.top_level.html '//pre[@class="rust item-decl"]' 'pub fn top_level(arg0: impl FnOnce())' | ||
| pub reuse foo as top_level; | ||
|
|
||
| pub struct S; | ||
|
|
||
| //@ has fn_delegation_impl_trait/struct.S.html '//*[@id="method.method"]' 'pub fn method(arg0: impl FnOnce())' | ||
| impl S { | ||
| pub reuse foo as method; | ||
| } | ||
|
|
||
| pub trait A { | ||
| fn f(&self, _: impl FnOnce()) {} | ||
| } | ||
|
|
||
| impl A for S {} | ||
|
|
||
| //@ has fn_delegation_impl_trait/struct.S.html '//*[@id="method.f"]' 'pub fn f(self: &S, arg1: impl FnOnce())' | ||
| //@ !has fn_delegation_impl_trait/struct.S.html '//*[@id="method.f"]' 'MetaSized' | ||
| impl S { | ||
| pub reuse <S as A>::f; | ||
| } | ||
|
|
||
| //@ has fn_delegation_impl_trait/trait.T.html '//*[@id="method.provided"]' 'fn provided(arg0: impl FnOnce())' | ||
| pub trait T { | ||
| reuse foo as provided; | ||
| } | ||
|
|
||
| //@ has fn_delegation_impl_trait/fn.cross_crate.html '//pre[@class="rust item-decl"]' 'pub fn cross_crate(arg0: impl FnOnce())' | ||
| pub reuse aux::external as cross_crate; | ||
|
|
||
| //@ has fn_delegation_impl_trait/fn.inlined_cross_crate_delegated.html '//pre[@class="rust item-decl"]' 'pub fn inlined_cross_crate_delegated(arg0: impl FnOnce())' | ||
| #[doc(inline)] | ||
| pub use aux::delegated as inlined_cross_crate_delegated; | ||
|
|
||
| //@ has fn_delegation_impl_trait/fn.redelegated_cross_crate.html '//pre[@class="rust item-decl"]' 'pub fn redelegated_cross_crate(arg0: impl FnOnce())' | ||
| pub reuse aux::delegated as redelegated_cross_crate; | ||
|
|
||
| pub fn main() {} | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're putting the HIR params first and the HIR where bounds last. this just seems needlessly confusing.