Skip to content

Commit

Permalink
Rollup merge of #97613 - jsha:implementation-is-on-local-type, r=Guil…
Browse files Browse the repository at this point in the history
…laumeGomez

rustdoc: Improve calculation of "Impls on Foreign Types"

The existing code to calculate whether an implementation was on a "Foreign Type" was duplicated across the sidebar generation and the page generation. It also came to the wrong conclusion for some cases where both the trait and the "for" type were re-exports.

This PR extracts the logic into a method of `Impl`, breaks it into a multi-line method so it can be commented, and adds a case for when the trait and the "for" type came from the same crate. This fixes some cases - like the platform-specific integer types (`__m256`, `__m128`, etc). But it doesn't fix all cases. See the screenshots below.

[Before](https://doc.rust-lang.org/nightly/std/clone/trait.Clone.html#foreign-impls):

<img src="https://user-images.githubusercontent.com/220205/171338226-59ce6daf-3d76-4bad-bc8d-72a8259a8f43.png" width=200>

[After](https://rustdoc.crud.net/jsha/implementation-is-on-local-type/std/clone/trait.Clone.html):

<img src="https://user-images.githubusercontent.com/220205/171338147-28308a65-1597-4223-be47-9550062404dd.png" width=200>

The remaining types (`CString`, `NulError`, etc) are all from the `alloc` crate, while the `Clone` trait is from the `core` crate. Since `CString` and `Clone` are both re-exported by `std`, they are logically local to each other, but I couldn't figure out a good way to detect that in this code. I figure this is still a good step forward.

Related: #97610

r? `@camelid`
  • Loading branch information
JohnTitor authored Jun 1, 2022
2 parents 2c1990d + 37d3638 commit a96e71c
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 7 deletions.
25 changes: 25 additions & 0 deletions src/librustdoc/formats/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rustc_hir::def_id::DefId;
pub(crate) use renderer::{run_format, FormatRenderer};

use crate::clean::{self, ItemId};
use cache::Cache;

/// Specifies whether rendering directly implemented trait items or ones from a certain Deref
/// impl.
Expand Down Expand Up @@ -60,4 +61,28 @@ impl Impl {
}
}
}

// Returns true if this is an implementation on a "local" type, meaning:
// the type is in the current crate, or the type and the trait are both
// re-exported by the current crate.
pub(crate) fn is_on_local_type(&self, cache: &Cache) -> bool {
let for_type = &self.inner_impl().for_;
if let Some(for_type_did) = for_type.def_id(cache) {
// The "for" type is local if it's in the paths for the current crate.
if cache.paths.contains_key(&for_type_did) {
return true;
}
if let Some(trait_did) = self.trait_did() {
// The "for" type and the trait are from the same crate. That could
// be different from the current crate, for instance when both were
// re-exported from some other crate. But they are local with respect to
// each other.
if for_type_did.krate == trait_did.krate {
return true;
}
}
return false;
};
true
}
}
4 changes: 1 addition & 3 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2285,9 +2285,7 @@ fn sidebar_trait(cx: &Context<'_>, buf: &mut Buffer, it: &clean::Item, t: &clean
if let Some(implementors) = cache.implementors.get(&it.item_id.expect_def_id()) {
let mut res = implementors
.iter()
.filter(|i| {
i.inner_impl().for_.def_id(cache).map_or(false, |d| !cache.paths.contains_key(&d))
})
.filter(|i| !i.is_on_local_type(cache))
.filter_map(|i| extract_for_impl_name(&i.impl_item, cx))
.collect::<Vec<_>>();

Expand Down
5 changes: 2 additions & 3 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,9 +822,8 @@ fn item_trait(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean:
}
}

let (local, foreign) = implementors.iter().partition::<Vec<_>, _>(|i| {
i.inner_impl().for_.def_id(cache).map_or(true, |d| cache.paths.contains_key(&d))
});
let (local, foreign) =
implementors.iter().partition::<Vec<_>, _>(|i| i.is_on_local_type(cache));

let (mut synthetic, mut concrete): (Vec<&&Impl>, Vec<&&Impl>) =
local.iter().partition(|i| i.inner_impl().kind.is_auto());
Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc/issue-75588.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ extern crate real_gimli;
// @!has foo/trait.Deref.html '//*[@id="impl-Deref-for-EndianSlice"]//h3[@class="code-header in-band"]' 'impl Deref for EndianSlice'
pub use realcore::Deref;

// @has foo/trait.Join.html '//*[@id="impl-Join-for-Foo"]//h3[@class="code-header in-band"]' 'impl Join for Foo'
// @has foo/trait.Join.html '//*[@id="impl-Join"]//h3[@class="code-header in-band"]' 'impl Join for Foo'
pub use realcore::Join;

0 comments on commit a96e71c

Please sign in to comment.