Skip to content
Closed
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
148 changes: 107 additions & 41 deletions src/librustdoc/clean/blanket_impl.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
use core::ops::ControlFlow;

use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::thin_vec::ThinVec;
use rustc_hir as hir;
use rustc_infer::infer::{DefineOpaqueTypes, InferOk, TyCtxtInferExt};
use rustc_infer::traits;
use rustc_middle::ty::{self, TypingMode, Unnormalized, Upcast};
use rustc_middle::ty::{
self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor, TypingMode, Unnormalized, Upcast,
};
use rustc_span::DUMMY_SP;
use rustc_span::def_id::DefId;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
Expand All @@ -14,6 +19,50 @@ use crate::clean::{
};
use crate::core::DocContext;

/// Detects recursive types to avoid infinite loops in blanket impl evaluation.
fn contains_recursive_type(tcx: TyCtxt<'_>, item_def_id: DefId) -> bool {

@fmease fmease Jul 30, 2026

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.

This is an extremely targeted fix that's also way too broad as it means that rustdoc stops showing blanket impls for all recursive data types which I don't consider acceptable.

The linked issue #160105 likely has the same root cause as #155759 and #114891.

The problem isn't with rustdoc's blanket impl synthesis, it's with the current trait solver. The next trait solver fixes this class of issues. Hence, I consider PR #125907 to be the principled fix (which is still blocked by some perf regressions caused by the switch).

What you're doing here is trying to patch faults of the trait solver from the outside just like PR #155765 which I'm also not a fan of.

View changes since the review

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 thanks for looking into it. I see the issue and i will love to have my time to look deeper into it.. and will inform as soon as i get something.

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 soo i dont see any way to fix this with the old trait solver. this must go under the new_trait_solver.. that you are already working on (#125907) here. soo you can close this PR..

let Some(adt_def) = tcx.type_of(item_def_id).skip_binder().ty_adt_def() else {
return false;
};

struct FindAdtVisitor<'tcx> {
target: DefId,
tcx: TyCtxt<'tcx>,
visited: FxHashSet<DefId>,
}

impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for FindAdtVisitor<'tcx> {
type Result = ControlFlow<()>;

fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result {
if let ty::Adt(adt_def, _) = t.kind() {
if adt_def.did() == self.target {
return ControlFlow::Break(());
}
if self.visited.insert(adt_def.did()) {
for field in adt_def.all_fields() {
let field_ty = self.tcx.type_of(field.did).skip_binder();
if self.visit_ty(field_ty).is_break() {
return ControlFlow::Break(());
}
}
}
}
t.super_visit_with(self)
}
}

let visited = FxHashSet::default();
let mut visitor = FindAdtVisitor { target: item_def_id, tcx, visited };
for field in adt_def.all_fields() {
let field_ty = tcx.type_of(field.did).skip_binder();
if visitor.visit_ty(field_ty).is_break() {
return true;
}
}
false
}

#[instrument(level = "debug", skip(cx))]
pub(crate) fn synthesize_blanket_impls(
cx: &mut DocContext<'_>,
Expand All @@ -22,6 +71,17 @@ pub(crate) fn synthesize_blanket_impls(
let tcx = cx.tcx;
let ty = tcx.type_of(item_def_id);

if contains_recursive_type(tcx, item_def_id) {
debug!("skipping blanket impls for recursive type {item_def_id:?}");
return Vec::new();
}

// Keep one infcx for all blanket impls on this type so the trait solver
// doesn't flush its caches between each one.
let infcx = tcx.infer_ctxt().build(TypingMode::non_body_analysis());
let args = infcx.fresh_args_for_item(DUMMY_SP, item_def_id);
let impl_ty = ty.instantiate(tcx, args).skip_norm_wip();

let mut blanket_impls = Vec::new();
for trait_def_id in tcx.visible_traits() {
if !cx.cache.effective_visibilities.is_reachable(tcx, trait_def_id)
Expand All @@ -31,53 +91,59 @@ pub(crate) fn synthesize_blanket_impls(
}
// NOTE: doesn't use `for_each_relevant_impl` to avoid looking at anything besides blanket impls
let trait_impls = tcx.trait_impls_of(trait_def_id);
'blanket_impls: for &impl_def_id in trait_impls.blanket_impls() {
for &impl_def_id in trait_impls.blanket_impls() {
trace!("considering impl `{impl_def_id:?}` for trait `{trait_def_id:?}`");

let trait_ref = tcx.impl_trait_ref(impl_def_id);
if !matches!(trait_ref.skip_binder().self_ty().kind(), ty::Param(_)) {
continue;
}
let infcx = tcx.infer_ctxt().build(TypingMode::non_body_analysis());
let args = infcx.fresh_args_for_item(DUMMY_SP, item_def_id);
let impl_ty = ty.instantiate(tcx, args).skip_norm_wip();
let param_env = ty::ParamEnv::empty();

let impl_args = infcx.fresh_args_for_item(DUMMY_SP, impl_def_id);
let impl_trait_ref = trait_ref.instantiate(tcx, impl_args).skip_norm_wip();

// Require the type the impl is implemented on to match
// our type, and ignore the impl if there was a mismatch.
let Ok(eq_result) = infcx.at(&traits::ObligationCause::dummy(), param_env).eq(
DefineOpaqueTypes::Yes,
impl_trait_ref.self_ty(),
impl_ty,
) else {
continue;
};
let InferOk { value: (), obligations } = eq_result;
// FIXME(eddyb) ignoring `obligations` might cause false positives.
drop(obligations);

let clauses = tcx
.clauses_of(impl_def_id)
.instantiate(tcx, impl_args)
.clauses
.into_iter()
.map(Unnormalized::skip_norm_wip)
.chain(Some(impl_trait_ref.upcast(tcx)));
for clause in clauses {
let obligation = traits::Obligation::new(
tcx,
traits::ObligationCause::dummy(),
param_env,
clause,
);
match infcx.evaluate_obligation(&obligation) {
Ok(eval_result) if eval_result.may_apply() => {}
Err(traits::OverflowError::Canonical) => {}
_ => continue 'blanket_impls,

// Roll back inference state per impl but keep the infcx alive
// so earlier evaluations still help with later ones.
let applies = infcx.probe(|_| {
let impl_args = infcx.fresh_args_for_item(DUMMY_SP, impl_def_id);
let impl_trait_ref = trait_ref.instantiate(tcx, impl_args).skip_norm_wip();
let param_env = ty::ParamEnv::empty();

// Require the type the impl is implemented on to match
// our type, and ignore the impl if there was a mismatch.
let Ok(eq_result) = infcx.at(&traits::ObligationCause::dummy(), param_env).eq(
DefineOpaqueTypes::Yes,
impl_trait_ref.self_ty(),
impl_ty,
) else {
return false;
};
let InferOk { value: (), obligations } = eq_result;
// FIXME(eddyb) ignoring `obligations` might cause false positives.
drop(obligations);

let clauses = tcx
.clauses_of(impl_def_id)
.instantiate(tcx, impl_args)
.clauses
.into_iter()
.map(Unnormalized::skip_norm_wip)
.chain(Some(impl_trait_ref.upcast(tcx)));
for clause in clauses {
let obligation = traits::Obligation::new(
tcx,
traits::ObligationCause::dummy(),
param_env,
clause,
);
match infcx.evaluate_obligation(&obligation) {
Ok(eval_result) if eval_result.may_apply() => {}
Err(traits::OverflowError::Canonical) => {}
_ => return false,
}
}
true
});

if !applies {
continue;
}
debug!("found applicable impl for trait ref {trait_ref:?}");

Expand Down
102 changes: 102 additions & 0 deletions tests/rustdoc-ui/blanket-impl-recursive-types-perf.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
//@ check-pass
// Regression test: blanket impls on recursive generic types caused
// `cargo doc` to take minutes instead of milliseconds.

use std::collections::{BTreeMap, BTreeSet};
use std::sync::Arc;

pub trait Erasable: Sync {}
impl<T> Erasable for T where T: Sync {}

pub trait SessionParameters {
type Verifier;
}

pub struct Node<T>(pub Arc<T>);

pub struct ComputeScalar<SP: SessionParameters> {
pub args: BTreeMap<String, ComputeScalarArg<SP>>,
pub dependencies: Dependency<SP>,
}

pub struct Collect<SP: SessionParameters> {
pub values: CollectArg<SP>,
pub dependencies: Dependency<SP>,
}

pub struct ComputeMapping<SP: SessionParameters> {
pub args: BTreeMap<String, ComputeMappingArg<SP>>,
pub dependencies: Dependency<SP>,
}

pub struct SendBC<SP: SessionParameters> {
pub data: Node<SerializeAndSignBC<SP>>,
pub destinations: BTreeSet<SP::Verifier>,
pub dependencies: Dependency<SP>,
}

pub struct SerializeAndSignBC<SP: SessionParameters> {
pub data: Node<ComputeScalar<SP>>,
pub dependencies: Dependency<SP>,
}

pub struct SerializeAndSignDM<SP: SessionParameters> {
pub data: DirectMessageArg<SP>,
pub dependencies: Dependency<SP>,
}

pub struct DeserializeAndCheck<SP: SessionParameters> {
pub data: Node<Receive<SP>>,
pub dependencies: Dependency<SP>,
}

pub struct SendDM<SP: SessionParameters> {
pub data: Node<SerializeAndSignDM<SP>>,
pub dependencies: Dependency<SP>,
}

pub struct Receive<SP: SessionParameters> {
pub dependencies: Dependency<SP>,
}

pub struct MergeScalars<SP: SessionParameters> {
pub left: ComputeScalarArg<SP>,
pub right: ComputeScalarArg<SP>,
}

pub enum ComputeScalarArg<SP: SessionParameters> {
ComputeScalar(Node<ComputeScalar<SP>>),
MergeScalars(Node<MergeScalars<SP>>),
Collect(Node<Collect<SP>>),
}

pub enum ComputeMappingArg<SP: SessionParameters> {
ComputeScalar(Node<ComputeScalar<SP>>),
MergeScalars(Node<MergeScalars<SP>>),
Collect(Node<Collect<SP>>),
ComputeMapping(Node<ComputeMapping<SP>>),
SerializeAndSignBC(Node<SerializeAndSignBC<SP>>),
SerializeAndSignDM(Node<SerializeAndSignDM<SP>>),
DeserializeAndCheck(Node<DeserializeAndCheck<SP>>),
}

pub enum CollectArg<SP: SessionParameters> {
ComputeMapping(Node<ComputeMapping<SP>>),
SerializeAndSign(Node<SerializeAndSignDM<SP>>),
DeserializeAndCheck(Node<DeserializeAndCheck<SP>>),
Send(Node<SendDM<SP>>),
Receive(Node<Receive<SP>>),
}

pub enum DirectMessageArg<SP: SessionParameters> {
ComputeScalar(Node<ComputeScalar<SP>>),
ComputeMapping(Node<ComputeMapping<SP>>),
DeserializeAndCheck(Node<DeserializeAndCheck<SP>>),
}

pub enum Dependency<SP: SessionParameters> {
ComputeScalar(Node<ComputeScalar<SP>>),
Collect(Node<Collect<SP>>),
MergeScalars(Node<MergeScalars<SP>>),
SendBC(Node<SendBC<SP>>),
}
Loading