Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,9 @@ pub(crate) fn build_impl(
};
let trait_ = associated_trait
.map(|t| clean_trait_ref_with_constraints(cx, ty::Binder::dummy(t), ThinVec::new()));
if trait_.as_ref().map(|t| t.def_id()) == tcx.lang_items().deref_trait() {
if trait_.as_ref().map(|t| t.def_id()) == tcx.lang_items().deref_trait()
&& polarity != ty::ImplPolarity::Negative
{
super::build_deref_target_impls(cx, &trait_items, ret);
}

Expand Down
6 changes: 4 additions & 2 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2941,9 +2941,11 @@ fn clean_impl<'tcx>(
.map(|&ii| clean_impl_item(tcx.hir_impl_item(ii), cx))
.collect::<Vec<_>>();

// If this impl block is an implementation of the Deref trait, then we
// If this impl block is a positive implementation of the Deref trait, then we
// need to try inlining the target's inherent impl blocks as well.
if trait_.as_ref().map(|t| t.def_id()) == tcx.lang_items().deref_trait() {
if trait_.as_ref().is_some_and(|t| tcx.lang_items().deref_trait() == Some(t.def_id()))
&& tcx.impl_polarity(def_id) != ty::ImplPolarity::Negative
{
build_deref_target_impls(cx, &items, &mut ret);
}

Expand Down
5 changes: 3 additions & 2 deletions src/librustdoc/html/render/mod.rs
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two build_impl fns (one in clean/mod.rs, the other in clean/mod.rs) also invoke extra code when encountering Deref trait impls by calling build_deref_target_impls. Likewise, in render/sidebar.rs we have two places that perform extra work for deref impls.

I'm relatively sure that all four of these places still process negative deref trait impls, likely unnecessarily so. If it's not too much work for you, could you filter out negative impls in these places (unless it's impossible for negative deref impls to show up there). It probably doesn't save that much or any processing time but it might be more robust.

If you do that it might also be worth adding a HTML test in tests/rustdoc-html to ensure that we still actually render impl !Deref for … impls (in the sidebar and the main container).

Copy link
Copy Markdown
Contributor Author

@jakubadamw jakubadamw Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fmease, thank you for reviewing the PR. 🙂

The two build_impl fns (one in clean/mod.rs, the other in clean/mod.rs)

I take it you mean clean/mod.rs and clean/inline.rs, as well as clean_impl and build_impl, respectively? I pushed a commit that addresses the suggestion for those four sites, and adds an HTML test. 🙂

Thanks again! Let me know if there’s anything else I’m missing here.

@rustbot ready

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fmease, would you be able to take a look at this again? I made the requested change. Thanks! 🙂

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two build_impl fns (one in clean/mod.rs, the other in clean/mod.rs)

I take it you mean clean/mod.rs and clean/inline.rs, as well as clean_impl and build_impl, respectively?

Ah yes, haha. Silly typo.

Thanks for addressing my feedback! Sorry, the PR slipped through the cracks because apparently rustbot still doesn't honor @rustbot ready in review comments, so this PR didn't show up using my usual filter.

Original file line number Diff line number Diff line change
Expand Up @@ -1517,8 +1517,9 @@ fn render_assoc_items_inner(
}

if !traits.is_empty() {
let deref_impl =
traits.iter().find(|t| t.trait_did() == cx.tcx().lang_items().deref_trait());
let deref_impl = traits.iter().find(|t| {
t.trait_did() == cx.tcx().lang_items().deref_trait() && !t.is_negative_trait_impl()
});
if let Some(impl_) = deref_impl {
let has_deref_mut =
traits.iter().any(|t| t.trait_did() == cx.tcx().lang_items().deref_mut_trait());
Expand Down
7 changes: 4 additions & 3 deletions src/librustdoc/html/render/sidebar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,9 +466,9 @@ fn sidebar_assoc_items<'a>(
];

if v.iter().any(|i| i.inner_impl().trait_.is_some()) {
if let Some(impl_) =
v.iter().find(|i| i.trait_did() == cx.tcx().lang_items().deref_trait())
{
if let Some(impl_) = v.iter().find(|i| {
i.trait_did() == cx.tcx().lang_items().deref_trait() && !i.is_negative_trait_impl()
}) {
let mut derefs = DefIdSet::default();
derefs.insert(did);
sidebar_deref_methods(
Expand Down Expand Up @@ -579,6 +579,7 @@ fn sidebar_deref_methods<'a>(
.as_ref()
.map(|t| Some(t.def_id()) == cx.tcx().lang_items().deref_trait())
.unwrap_or(false)
&& !i.is_negative_trait_impl()
})
{
sidebar_deref_methods(
Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/passes/collect_trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,9 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) ->

// scan through included items ahead of time to splice in Deref targets to the "valid" sets
for it in new_items_external.iter().chain(new_items_local.iter()) {
if let ImplItem(box Impl { ref for_, ref trait_, ref items, .. }) = it.kind
if let ImplItem(box Impl { ref for_, ref trait_, ref items, polarity, .. }) = it.kind
&& trait_.as_ref().map(|t| t.def_id()) == tcx.lang_items().deref_trait()
&& polarity != ty::ImplPolarity::Negative
&& cleaner.keep_impl(for_, true)
{
let target = items
Expand Down
23 changes: 23 additions & 0 deletions tests/rustdoc-html/deref/negative-deref-impl-128801.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#![feature(negative_impls)]
#![crate_name = "foo"]

// Regression test for https://github.com/rust-lang/rust/issues/128801
// Negative `Deref`/`DerefMut` impls should not cause an ICE and should still be rendered.

pub struct Source;

//@ has foo/struct.Source.html

// Verify negative Deref impl is rendered in the main content.
//@ has - '//*[@class="impl"]//h3[@class="code-header"]' 'impl !Deref for Source'

// Verify negative DerefMut impl is rendered in the main content.
//@ has - '//*[@class="impl"]//h3[@class="code-header"]' 'impl !DerefMut for Source'

// Verify negative impls appear in the sidebar.
//@ has - '//div[@class="sidebar-elems"]//h3/a[@href="#trait-implementations"]' 'Trait Implementations'
//@ has - '//*[@class="sidebar-elems"]//section//a' '!Deref'
//@ has - '//*[@class="sidebar-elems"]//section//a' '!DerefMut'

impl !std::ops::Deref for Source {}
impl !std::ops::DerefMut for Source {}
11 changes: 11 additions & 0 deletions tests/rustdoc-ui/deref/negative-deref-ice-128801.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//@ check-pass

// Regression test for https://github.com/rust-lang/rust/issues/128801
// Negative `Deref`/`DerefMut` impls should not cause an ICE.

#![feature(negative_impls)]

pub struct Source;

impl !std::ops::Deref for Source {}
impl !std::ops::DerefMut for Source {}
Loading