-
-
Notifications
You must be signed in to change notification settings - Fork 15.1k
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
|
cijiugechu marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,20 +136,25 @@ pub(crate) fn sized_bounds(cx: &mut DocContext<'_>, generics: &mut clean::Generi | |
| // Note that associated types also have an implicit `Sized` bound but we | ||
| // don't actually know the set of associated types right here so that | ||
| // should be handled when cleaning associated types. | ||
| generics.where_predicates.retain(|pred| { | ||
| let WP::BoundPredicate { ty: clean::Generic(param), bounds, .. } = pred else { | ||
| generics.where_predicates.retain_mut(|pred| { | ||
| let WP::BoundPredicate { ty, bounds, .. } = pred else { | ||
| return true; | ||
| }; | ||
|
|
||
| // FIXME(sized-hierarchy): Always skip `MetaSized` bounds so that only `?Sized` | ||
| // is shown and none of the new sizedness traits leak into documentation. | ||
| bounds.retain(|b| !b.is_meta_sized_bound(cx.tcx)); | ||
|
Member
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. Can |
||
|
|
||
| let clean::Generic(param) = ty else { | ||
| return !bounds.is_empty(); | ||
| }; | ||
|
|
||
| if bounds.iter().any(|b| b.is_sized_bound(cx.tcx)) { | ||
| sized_params.insert(*param); | ||
| false | ||
| } else if bounds.iter().any(|b| b.is_meta_sized_bound(cx.tcx)) { | ||
| // FIXME(sized-hierarchy): Always skip `MetaSized` bounds so that only `?Sized` | ||
| // is shown and none of the new sizedness traits leak into documentation. | ||
| false | ||
| bounds.retain(|b| !b.is_sized_bound(cx.tcx)); | ||
|
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 looks like a fix for an unrelated bug, could also make a regression test for that bug? |
||
| !bounds.is_empty() | ||
| } else { | ||
| true | ||
| !bounds.is_empty() | ||
| } | ||
| }); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,10 @@ | ||
| // This test ensures that when clicking on a link which leads to an item inside a collapsed element, | ||
| // the collapsed element will be expanded. | ||
|
|
||
| // We need to disable this check because `trait.impl/lib2/scroll_traits/trait.Iterator.js` | ||
| // doesn't exist. | ||
| fail-on-request-error: false | ||
|
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. why does it not exist? is this a bug? if so it at least deserves a FIXME, if not a github issue. |
||
|
|
||
| go-to: "file://" + |DOC_PATH| + "/test_docs/struct.Foo.html" | ||
| // We check that the implementors block is expanded. | ||
| assert-property: ("#implementations-list .implementors-toggle", {"open": "true"}) | ||
|
|
||
| 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,48 @@ | ||||||
| // 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:fn_delegation_impl_trait_aux=fn-delegation-impl-trait-aux.rs | ||||||
|
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.
Suggested change
this should allow us to completely remove the |
||||||
| #![feature(fn_delegation)] | ||||||
| #![allow(incomplete_features)] | ||||||
|
|
||||||
|
cijiugechu marked this conversation as resolved.
|
||||||
| extern crate fn_delegation_impl_trait_aux as aux; | ||||||
|
|
||||||
| 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.
does this code reorder generic parameters under certain situations? it would be problematic if it did, since the order of generic parameters is important when using turbofish syntax. Even if there's no situation where this can cause problems currently, substituting the placeholders in a single pass would likely be more future proof than removing then re-adding them.