Skip to content

Commit b46412f

Browse files
rustdoc: be more strict about "Methods from Deref"
hack: is_doc_subtype_of always returns true for TyAlias it's worth noting that this function is only used in the handling of "Methods from Deref", and we were previously assuming all generic parameters were meaningless, so this is still an improvment from the status quo. this change means that we will have strictly less false positives without adding any new false negitives. Co-authored-by: Guillaume Gomez <guillaume1.gomez@gmail.com>
1 parent d497e43 commit b46412f

File tree

4 files changed

+59
-3
lines changed

4 files changed

+59
-3
lines changed

src/librustdoc/clean/types.rs

+14
Original file line numberDiff line numberDiff line change
@@ -1533,6 +1533,10 @@ impl Type {
15331533
matches!(self, Type::BorrowedRef { .. })
15341534
}
15351535

1536+
fn is_type_alias(&self) -> bool {
1537+
matches!(self, Type::Path { path: Path { res: Res::Def(DefKind::TyAlias, _), .. } })
1538+
}
1539+
15361540
/// Check if two types are "the same" for documentation purposes.
15371541
///
15381542
/// This is different from `Eq`, because it knows that things like
@@ -1561,6 +1565,16 @@ impl Type {
15611565
} else {
15621566
(self, other)
15631567
};
1568+
1569+
// FIXME: `Cache` does not have the data required to unwrap type aliases,
1570+
// so we just assume they are equal.
1571+
// This is only remotely acceptable because we were previously
1572+
// assuming all types were equal when used
1573+
// as a generic parameter of a type in `Deref::Target`.
1574+
if self_cleared.is_type_alias() || other_cleared.is_type_alias() {
1575+
return true;
1576+
}
1577+
15641578
match (self_cleared, other_cleared) {
15651579
// Recursive cases.
15661580
(Type::Tuple(a), Type::Tuple(b)) => {

src/librustdoc/html/render/mod.rs

+14-2
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ pub(crate) fn ensure_trailing_slash(v: &str) -> impl fmt::Display {
8989

9090
/// Specifies whether rendering directly implemented trait items or ones from a certain Deref
9191
/// impl.
92-
#[derive(Copy, Clone)]
92+
#[derive(Copy, Clone, Debug)]
9393
pub(crate) enum AssocItemRender<'a> {
9494
All,
9595
DerefFor { trait_: &'a clean::Path, type_: &'a clean::Type, deref_mut_: bool },
@@ -1296,7 +1296,8 @@ fn render_assoc_items_inner(
12961296
info!("Documenting associated items of {:?}", containing_item.name);
12971297
let cache = &cx.shared.cache;
12981298
let Some(v) = cache.impls.get(&it) else { return };
1299-
let (non_trait, traits): (Vec<_>, _) = v.iter().partition(|i| i.inner_impl().trait_.is_none());
1299+
let (mut non_trait, traits): (Vec<_>, _) =
1300+
v.iter().partition(|i| i.inner_impl().trait_.is_none());
13001301
if !non_trait.is_empty() {
13011302
let mut close_tags = <Vec<&str>>::with_capacity(1);
13021303
let mut tmp_buf = String::new();
@@ -1314,6 +1315,16 @@ fn render_assoc_items_inner(
13141315
AssocItemRender::DerefFor { trait_, type_, deref_mut_ } => {
13151316
let id =
13161317
cx.derive_id(small_url_encode(format!("deref-methods-{:#}", type_.print(cx))));
1318+
// the `impls.get` above only looks at the outermost type,
1319+
// and the Deref impl may only be implemented for certain
1320+
// values of generic parameters.
1321+
// for example, if an item impls `Deref<[u8]>`,
1322+
// we should not show methods from `[MaybeUninit<u8>]`.
1323+
// this `retain` filters out any instances where
1324+
// the types do not line up perfectly.
1325+
non_trait.retain(|impl_| {
1326+
type_.is_doc_subtype_of(&impl_.inner_impl().for_, &cx.shared.cache)
1327+
});
13171328
let derived_id = cx.derive_id(&id);
13181329
close_tags.push("</details>");
13191330
write_str(
@@ -1392,6 +1403,7 @@ fn render_assoc_items_inner(
13921403
}
13931404
}
13941405

1406+
/// `derefs` is the set of all deref targets that have already been handled.
13951407
fn render_deref_methods(
13961408
mut w: impl Write,
13971409
cx: &Context<'_>,

src/librustdoc/html/render/sidebar.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,10 @@ fn sidebar_deref_methods<'a>(
533533
debug!("found inner_impl: {impls:?}");
534534
let mut ret = impls
535535
.iter()
536-
.filter(|i| i.inner_impl().trait_.is_none())
536+
.filter(|i| {
537+
i.inner_impl().trait_.is_none()
538+
&& real_target.is_doc_subtype_of(&i.inner_impl().for_, &c)
539+
})
537540
.flat_map(|i| get_methods(i.inner_impl(), true, used_links, deref_mut, cx.tcx()))
538541
.collect::<Vec<_>>();
539542
if !ret.is_empty() {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#![crate_name = "foo"]
2+
3+
// test for https://github.com/rust-lang/rust/issues/24686
4+
use std::ops::Deref;
5+
6+
pub struct Foo<T>(T);
7+
impl Foo<i32> {
8+
pub fn get_i32(&self) -> i32 { self.0 }
9+
}
10+
impl Foo<u32> {
11+
pub fn get_u32(&self) -> u32 { self.0 }
12+
}
13+
14+
// Note that the same href is used both on the method itself,
15+
// and on the sidebar items.
16+
//@ has foo/struct.Bar.html
17+
//@ has - '//a[@href="#method.get_i32"]' 'get_i32'
18+
//@ !has - '//a[@href="#method.get_u32"]' 'get_u32'
19+
//@ count - '//ul[@class="block deref-methods"]//a' 1
20+
//@ count - '//a[@href="#method.get_i32"]' 2
21+
pub struct Bar(Foo<i32>);
22+
impl Deref for Bar {
23+
type Target = Foo<i32>;
24+
fn deref(&self) -> &Foo<i32> {
25+
&self.0
26+
}
27+
}

0 commit comments

Comments
 (0)