Skip to content

Commit

Permalink
Unrolled build for rust-lang#119997
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#119997 - GuillaumeGomez:fix-stripped-impl-on-ty-alias, r=notriddle

Fix impl stripped in rustdoc HTML whereas it should not be in case the impl is implemented on a type alias

Fixes rust-lang#119015.

I talked about it a bit with ```@petrochenkov.``` They might change what `EffectiveVisibilities` return for impl items like this one and make them not only reachable but also re-exported, which would fix this case. It could also potentially break other things, so it'll be done whenever they can and then we can check together.

Surprisingly, this fix is making rustdoc even closer to rustc in term of errors (the CI currently fails because currently accepted broken codes aren't working anymore with this change). Not sure exactly why though. This is linked to rust-lang#110631 from what I could find.

So either I'm missing something here, or we consider it's ok and we consider the failing tests as "should fail" and I'll update `rustdoc-ui` ones.

r? ```@notriddle```
  • Loading branch information
rust-timer authored Jan 20, 2024
2 parents 1828461 + 0933f48 commit b503ef9
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 19 deletions.
31 changes: 19 additions & 12 deletions src/librustdoc/passes/stripper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,10 @@ impl<'a, 'tcx> DocFolder for Stripper<'a, 'tcx> {
| clean::TraitItem(..)
| clean::FunctionItem(..)
| clean::VariantItem(..)
| clean::MethodItem(..)
| clean::ForeignFunctionItem(..)
| clean::ForeignStaticItem(..)
| clean::ConstantItem(..)
| clean::UnionItem(..)
| clean::AssocConstItem(..)
| clean::AssocTypeItem(..)
| clean::TraitAliasItem(..)
| clean::MacroItem(..)
| clean::ForeignTypeItem => {
Expand All @@ -80,6 +77,16 @@ impl<'a, 'tcx> DocFolder for Stripper<'a, 'tcx> {
}
}

clean::MethodItem(..) | clean::AssocConstItem(..) | clean::AssocTypeItem(..) => {
let item_id = i.item_id;
if item_id.is_local()
&& !self.effective_visibilities.is_reachable(self.tcx, item_id.expect_def_id())
{
debug!("Stripper: stripping {:?} {:?}", i.type_(), i.name);
return None;
}
}

clean::StructFieldItem(..) => {
if i.visibility(self.tcx) != Some(Visibility::Public) {
return Some(strip_item(i));
Expand Down Expand Up @@ -192,16 +199,16 @@ impl<'a> DocFolder for ImplStripper<'a, '_> {
&& imp.items.iter().all(|i| {
let item_id = i.item_id;
item_id.is_local()
&& !is_item_reachable(
self.tcx,
self.is_json_output,
&self.cache.effective_visibilities,
item_id,
)
&& !self
.cache
.effective_visibilities
.is_reachable(self.tcx, item_id.expect_def_id())
})
{
debug!("ImplStripper: no public item; removing {imp:?}");
return None;
} else if imp.items.is_empty() && i.doc_value().is_empty() {
debug!("ImplStripper: no item and no doc; removing {imp:?}");
return None;
}
}
Expand All @@ -212,21 +219,21 @@ impl<'a> DocFolder for ImplStripper<'a, '_> {
&& !imp.for_.is_assoc_ty()
&& !self.should_keep_impl(&i, did)
{
debug!("ImplStripper: impl item for stripped type; removing");
debug!("ImplStripper: impl item for stripped type; removing {imp:?}");
return None;
}
if let Some(did) = imp.trait_.as_ref().map(|t| t.def_id())
&& !self.should_keep_impl(&i, did)
{
debug!("ImplStripper: impl item for stripped trait; removing");
debug!("ImplStripper: impl item for stripped trait; removing {imp:?}");
return None;
}
if let Some(generics) = imp.trait_.as_ref().and_then(|t| t.generics()) {
for typaram in generics {
if let Some(did) = typaram.def_id(self.cache)
&& !self.should_keep_impl(&i, did)
{
debug!("ImplStripper: stripped item in trait's generics; removing impl");
debug!("ImplStripper: stripped item in trait's generics; removing {imp:?}");
return None;
}
}
Expand Down
18 changes: 11 additions & 7 deletions tests/rustdoc/async-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ impl Foo {

pub trait Pattern<'a> {}

impl Pattern<'_> for () {}

pub trait Trait<const N: usize> {}
// @has async_fn/fn.const_generics.html
// @has - '//pre[@class="rust item-decl"]' 'pub async fn const_generics<const N: usize>(_: impl Trait<N>)'
Expand All @@ -57,18 +59,18 @@ pub async fn const_generics<const N: usize>(_: impl Trait<N>) {}
// regression test for #63037
// @has async_fn/fn.elided.html
// @has - '//pre[@class="rust item-decl"]' 'pub async fn elided(foo: &str) -> &str'
pub async fn elided(foo: &str) -> &str {}
pub async fn elided(foo: &str) -> &str { "" }
// This should really be shown as written, but for implementation reasons it's difficult.
// See `impl Clean for TyKind::Ref`.
// @has async_fn/fn.user_elided.html
// @has - '//pre[@class="rust item-decl"]' 'pub async fn user_elided(foo: &str) -> &str'
pub async fn user_elided(foo: &'_ str) -> &str {}
pub async fn user_elided(foo: &'_ str) -> &str { "" }
// @has async_fn/fn.static_trait.html
// @has - '//pre[@class="rust item-decl"]' 'pub async fn static_trait(foo: &str) -> Box<dyn Bar>'
pub async fn static_trait(foo: &str) -> Box<dyn Bar> {}
pub async fn static_trait(foo: &str) -> Box<dyn Bar> { Box::new(()) }
// @has async_fn/fn.lifetime_for_trait.html
// @has - '//pre[@class="rust item-decl"]' "pub async fn lifetime_for_trait(foo: &str) -> Box<dyn Bar + '_>"
pub async fn lifetime_for_trait(foo: &str) -> Box<dyn Bar + '_> {}
pub async fn lifetime_for_trait(foo: &str) -> Box<dyn Bar + '_> { Box::new(()) }
// @has async_fn/fn.elided_in_input_trait.html
// @has - '//pre[@class="rust item-decl"]' "pub async fn elided_in_input_trait(t: impl Pattern<'_>)"
pub async fn elided_in_input_trait(t: impl Pattern<'_>) {}
Expand All @@ -78,18 +80,20 @@ struct AsyncFdReadyGuard<'a, T> { x: &'a T }
impl Foo {
// @has async_fn/struct.Foo.html
// @has - '//*[@class="method"]' 'pub async fn complicated_lifetimes( &self, context: &impl Bar ) -> impl Iterator<Item = &usize>'
pub async fn complicated_lifetimes(&self, context: &impl Bar) -> impl Iterator<Item = &usize> {}
pub async fn complicated_lifetimes(&self, context: &impl Bar) -> impl Iterator<Item = &usize> {
[0].iter()
}
// taken from `tokio` as an example of a method that was particularly bad before
// @has - '//*[@class="method"]' "pub async fn readable<T>(&self) -> Result<AsyncFdReadyGuard<'_, T>, ()>"
pub async fn readable<T>(&self) -> Result<AsyncFdReadyGuard<'_, T>, ()> {}
pub async fn readable<T>(&self) -> Result<AsyncFdReadyGuard<'_, T>, ()> { Err(()) }
// @has - '//*[@class="method"]' "pub async fn mut_self(&mut self)"
pub async fn mut_self(&mut self) {}
}

// test named lifetimes, just in case
// @has async_fn/fn.named.html
// @has - '//pre[@class="rust item-decl"]' "pub async fn named<'a, 'b>(foo: &'a str) -> &'b str"
pub async fn named<'a, 'b>(foo: &'a str) -> &'b str {}
pub async fn named<'a, 'b>(foo: &'a str) -> &'b str { "" }
// @has async_fn/fn.named_trait.html
// @has - '//pre[@class="rust item-decl"]' "pub async fn named_trait<'a, 'b>(foo: impl Pattern<'a>) -> impl Pattern<'b>"
pub async fn named_trait<'a, 'b>(foo: impl Pattern<'a>) -> impl Pattern<'b> {}
27 changes: 27 additions & 0 deletions tests/rustdoc/impl-on-ty-alias-issue-119015.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#![crate_name = "foo"]

// @has 'foo/index.html'
// There should be only `type A`.
// @count - '//*[@class="item-table"]//*[@class="item-name"]' 1
// @has - '//*[@class="item-name"]/a[@href="type.A.html"]' 'A'

mod foo {
pub struct S;
}

use foo::S;

pub type A = S;

// @has 'foo/type.A.html'
// @has - '//*[@id="method.default"]/h4' 'fn default() -> Self'
impl Default for A {
fn default() -> Self {
S
}
}

// @has - '//*[@id="method.a"]/h4' 'pub fn a(&self)'
impl A {
pub fn a(&self) {}
}

0 comments on commit b503ef9

Please sign in to comment.