-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
Avoid recursive blanket impl checks #155765
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ 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, TypingMode, Unnormalized}; | ||
| use rustc_span::DUMMY_SP; | ||
| use rustc_span::def_id::DefId; | ||
| use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; | ||
|
|
@@ -21,11 +21,12 @@ pub(crate) fn synthesize_blanket_impls( | |
| ) -> Vec<clean::Item> { | ||
| let tcx = cx.tcx; | ||
| let ty = tcx.type_of(item_def_id); | ||
| let item_ty = ty.instantiate_identity().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) | ||
| || cx.generated_synthetics.contains(&(ty.skip_binder(), trait_def_id)) | ||
| || cx.generated_synthetics.contains(&(item_ty, trait_def_id)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
@@ -59,14 +60,26 @@ pub(crate) fn synthesize_blanket_impls( | |
| // FIXME(eddyb) ignoring `obligations` might cause false positives. | ||
| drop(obligations); | ||
|
|
||
| // Only the impl's where-predicates need to be checked here. The trait ref itself is | ||
| // the blanket impl candidate we're considering; proving it would select recursively. | ||
| let predicates = tcx | ||
| .predicates_of(impl_def_id) | ||
| .instantiate(tcx, impl_args) | ||
| .predicates | ||
| .into_iter() | ||
| .map(Unnormalized::skip_norm_wip) | ||
| .chain(Some(impl_trait_ref.upcast(tcx))); | ||
| .map(Unnormalized::skip_norm_wip); | ||
| for predicate in predicates { | ||
| let predicate = infcx.resolve_vars_if_possible(predicate); | ||
| if let Some(may_apply) = | ||
| cached_auto_trait_predicate_result(cx, predicate, impl_ty, item_ty) | ||
|
Comment on lines
+72
to
+74
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's very odd to me that we have to special case auto traits here when synthesizing blanket impls, it feels like we're manually helping out the trait solver here when it's likely the old trait solver's fault? Well, I don't actually know the actually fast-expanding tree of obligations in dtolnay's example. Could you enlighten me?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm gonna rebase my dusty PR #125907, impl lcnr's final suggestion and check if that PR also fixes the issue.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #125907 does indeed fix the issue, too.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, so next solver should be a better fix for it. |
||
| { | ||
| if may_apply { | ||
| continue; | ||
| } else { | ||
| continue 'blanket_impls; | ||
| } | ||
| } | ||
|
|
||
| let obligation = traits::Obligation::new( | ||
| tcx, | ||
| traits::ObligationCause::dummy(), | ||
|
|
@@ -81,7 +94,7 @@ pub(crate) fn synthesize_blanket_impls( | |
| } | ||
| debug!("found applicable impl for trait ref {trait_ref:?}"); | ||
|
|
||
| cx.generated_synthetics.insert((ty.skip_binder(), trait_def_id)); | ||
| cx.generated_synthetics.insert((item_ty, trait_def_id)); | ||
|
|
||
| blanket_impls.push(clean::Item { | ||
| inner: Box::new(clean::ItemInner { | ||
|
|
@@ -133,3 +146,31 @@ pub(crate) fn synthesize_blanket_impls( | |
|
|
||
| blanket_impls | ||
| } | ||
|
|
||
| fn cached_auto_trait_predicate_result<'tcx>( | ||
| cx: &DocContext<'tcx>, | ||
| predicate: ty::Clause<'tcx>, | ||
| impl_ty: Ty<'tcx>, | ||
| item_ty: Ty<'tcx>, | ||
| ) -> Option<bool> { | ||
| let trait_pred = predicate.as_trait_clause()?; | ||
| if trait_pred.polarity() != ty::PredicatePolarity::Positive { | ||
| return None; | ||
| } | ||
|
|
||
| let trait_def_id = trait_pred.def_id(); | ||
| if !cx.tcx.trait_is_auto(trait_def_id) { | ||
| return None; | ||
| } | ||
|
|
||
| let self_ty = trait_pred.self_ty().no_bound_vars()?; | ||
| if self_ty != impl_ty { | ||
| return None; | ||
| } | ||
|
|
||
| match cx.generated_auto_trait_impls.get(&(item_ty, trait_def_id)) { | ||
| Some(ty::ImplPolarity::Positive) => Some(true), | ||
| Some(ty::ImplPolarity::Negative) => Some(false), | ||
| _ => None, | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,8 @@ pub(crate) struct DocContext<'tcx> { | |
| /// Auto-trait or blanket impls processed so far, as `(self_ty, trait_def_id)`. | ||
| // FIXME(eddyb) make this a `ty::TraitRef<'tcx>` set. | ||
| pub(crate) generated_synthetics: FxHashSet<(Ty<'tcx>, DefId)>, | ||
| /// Polarity of synthesized auto-trait impls processed so far. | ||
| pub(crate) generated_auto_trait_impls: FxHashMap<(Ty<'tcx>, DefId), ty::ImplPolarity>, | ||
|
Comment on lines
61
to
+63
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't try to address this now since I want to test my other PR first. I'd just like to note that it's confusing to name this |
||
| pub(crate) auto_traits: Vec<DefId>, | ||
| /// This same cache is used throughout rustdoc, including in [`crate::html::render`]. | ||
| pub(crate) cache: Cache, | ||
|
|
@@ -370,6 +372,7 @@ pub(crate) fn run_global_ctxt( | |
| current_type_aliases: Default::default(), | ||
| impl_trait_bounds: Default::default(), | ||
| generated_synthetics: Default::default(), | ||
| generated_auto_trait_impls: Default::default(), | ||
| auto_traits, | ||
| cache: Cache::new(render_options.document_private, render_options.document_hidden), | ||
| inlined: FxHashSet::default(), | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can just be a rustdoc UI test since we don't care about the HTML. For that move, it into |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,268 @@ | ||
| #![crate_name = "foo"] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In a source code comment at the top, can you add which initial (auto trait, ADT) pairing(s) used to cause the blowout (unless that info isn't meaningful) |
||
|
|
||
| //@ has 'foo/struct.Class.html' | ||
| //@ has - '//*[@id="impl-ThreadSafe-for-T"]' 'impl<T> ThreadSafe for T' | ||
| pub struct Class<Span> { | ||
| pub body: std::sync::Arc<[stmt::Statement<Span>]>, | ||
| pub decorators: std::sync::Arc<[expr::Expression<Span>]>, | ||
| pub span: Span, | ||
| } | ||
|
|
||
| pub trait ThreadSafe: Send + Sync {} | ||
|
|
||
| // With this line commented out, the slow behavior does not occur. | ||
| impl<T: Send + Sync> ThreadSafe for T {} | ||
|
|
||
| mod stmt { | ||
| pub struct S00<Span>(pub Statement<Span>); | ||
| pub struct S01<Span>(pub Statement<Span>); | ||
| pub struct S02<Span>(pub Statement<Span>); | ||
| pub struct S03<Span>(pub Statement<Span>); | ||
| pub struct S04<Span>(pub Statement<Span>); | ||
| pub struct S05<Span>(pub Statement<Span>); | ||
| pub struct S06<Span>(pub Statement<Span>); | ||
| pub struct S07<Span>(pub Statement<Span>); | ||
| pub struct S08<Span>(pub Statement<Span>); | ||
| pub struct S09<Span>(pub Statement<Span>); | ||
| pub struct S10<Span>(pub Statement<Span>); | ||
| pub struct S11<Span>(pub Statement<Span>); | ||
| pub struct S12<Span>(pub Statement<Span>); | ||
| pub struct S13<Span>(pub Statement<Span>); | ||
| pub struct S14<Span>(pub Statement<Span>); | ||
| pub struct S15<Span>(pub Statement<Span>); | ||
| pub struct S16<Span>(pub Statement<Span>); | ||
| pub struct S17<Span>(pub Statement<Span>); | ||
| pub struct S18<Span>(pub Statement<Span>); | ||
| pub struct S19<Span>(pub Statement<Span>); | ||
| pub struct S20<Span>(pub Statement<Span>); | ||
| pub struct S21<Span>(pub Statement<Span>); | ||
| pub struct S22<Span>(pub Statement<Span>); | ||
| pub struct S23<Span>(pub Statement<Span>); | ||
| pub struct S24<Span>(pub Statement<Span>); | ||
| pub struct S25<Span>(pub Statement<Span>); | ||
| pub struct S26<Span>(pub Statement<Span>); | ||
| pub struct S27<Span>(pub Statement<Span>); | ||
| pub struct S28<Span>(pub Statement<Span>); | ||
| pub struct S29<Span>(pub Statement<Span>); | ||
| pub struct S30<Span>(pub Statement<Span>); | ||
| pub struct S31<Span>(pub Statement<Span>); | ||
| pub struct S32<Span>(pub Statement<Span>); | ||
| pub struct S33<Span>(pub Statement<Span>); | ||
| pub struct S34<Span>(pub Statement<Span>); | ||
| pub struct S35<Span>(pub Statement<Span>); | ||
| pub struct S36<Span>(pub Statement<Span>); | ||
| pub struct S37<Span>(pub Statement<Span>); | ||
| pub struct S38<Span>(pub Statement<Span>); | ||
| pub struct S39<Span>(pub Statement<Span>); | ||
| pub struct S40<Span>(pub Statement<Span>); | ||
| pub struct S41<Span>(pub Statement<Span>); | ||
| pub struct S42<Span>(pub Statement<Span>); | ||
| pub struct S43<Span>(pub Statement<Span>); | ||
| pub struct S44<Span>(pub Statement<Span>); | ||
| pub struct S45<Span>(pub Statement<Span>); | ||
| pub struct S46<Span>(pub Statement<Span>); | ||
| pub struct S47<Span>(pub Statement<Span>); | ||
| pub struct S48<Span>(pub Statement<Span>); | ||
| pub struct S49<Span>(pub Statement<Span>); | ||
| pub struct S50<Span>(pub Statement<Span>); | ||
| pub struct S51<Span>(pub Statement<Span>); | ||
| pub struct S52<Span>(pub Statement<Span>); | ||
| pub struct S53<Span>(pub Statement<Span>); | ||
| pub struct S54<Span>(pub Statement<Span>); | ||
| pub struct S55<Span>(pub Statement<Span>); | ||
| pub struct S56<Span>(pub Statement<Span>); | ||
| pub struct S57<Span>(pub Statement<Span>); | ||
| pub struct S58<Span>(pub Statement<Span>); | ||
| pub struct S59<Span>(pub Statement<Span>); | ||
|
|
||
| pub enum Statement<Span> { | ||
| Class(std::sync::Arc<crate::Class<Span>>), | ||
| S00(std::sync::Arc<S00<Span>>), | ||
| S01(std::sync::Arc<S01<Span>>), | ||
| S02(std::sync::Arc<S02<Span>>), | ||
| S03(std::sync::Arc<S03<Span>>), | ||
| S04(std::sync::Arc<S04<Span>>), | ||
| S05(std::sync::Arc<S05<Span>>), | ||
| S06(std::sync::Arc<S06<Span>>), | ||
| S07(std::sync::Arc<S07<Span>>), | ||
| S08(std::sync::Arc<S08<Span>>), | ||
| S09(std::sync::Arc<S09<Span>>), | ||
| S10(std::sync::Arc<S10<Span>>), | ||
| S11(std::sync::Arc<S11<Span>>), | ||
| S12(std::sync::Arc<S12<Span>>), | ||
| S13(std::sync::Arc<S13<Span>>), | ||
| S14(std::sync::Arc<S14<Span>>), | ||
| S15(std::sync::Arc<S15<Span>>), | ||
| S16(std::sync::Arc<S16<Span>>), | ||
| S17(std::sync::Arc<S17<Span>>), | ||
| S18(std::sync::Arc<S18<Span>>), | ||
| S19(std::sync::Arc<S19<Span>>), | ||
| S20(std::sync::Arc<S20<Span>>), | ||
| S21(std::sync::Arc<S21<Span>>), | ||
| S22(std::sync::Arc<S22<Span>>), | ||
| S23(std::sync::Arc<S23<Span>>), | ||
| S24(std::sync::Arc<S24<Span>>), | ||
| S25(std::sync::Arc<S25<Span>>), | ||
| S26(std::sync::Arc<S26<Span>>), | ||
| S27(std::sync::Arc<S27<Span>>), | ||
| S28(std::sync::Arc<S28<Span>>), | ||
| S29(std::sync::Arc<S29<Span>>), | ||
| S30(std::sync::Arc<S30<Span>>), | ||
| S31(std::sync::Arc<S31<Span>>), | ||
| S32(std::sync::Arc<S32<Span>>), | ||
| S33(std::sync::Arc<S33<Span>>), | ||
| S34(std::sync::Arc<S34<Span>>), | ||
| S35(std::sync::Arc<S35<Span>>), | ||
| S36(std::sync::Arc<S36<Span>>), | ||
| S37(std::sync::Arc<S37<Span>>), | ||
| S38(std::sync::Arc<S38<Span>>), | ||
| S39(std::sync::Arc<S39<Span>>), | ||
| S40(std::sync::Arc<S40<Span>>), | ||
| S41(std::sync::Arc<S41<Span>>), | ||
| S42(std::sync::Arc<S42<Span>>), | ||
| S43(std::sync::Arc<S43<Span>>), | ||
| S44(std::sync::Arc<S44<Span>>), | ||
| S45(std::sync::Arc<S45<Span>>), | ||
| S46(std::sync::Arc<S46<Span>>), | ||
| S47(std::sync::Arc<S47<Span>>), | ||
| S48(std::sync::Arc<S48<Span>>), | ||
| S49(std::sync::Arc<S49<Span>>), | ||
| S50(std::sync::Arc<S50<Span>>), | ||
| S51(std::sync::Arc<S51<Span>>), | ||
| S52(std::sync::Arc<S52<Span>>), | ||
| S53(std::sync::Arc<S53<Span>>), | ||
| S54(std::sync::Arc<S54<Span>>), | ||
| S55(std::sync::Arc<S55<Span>>), | ||
| S56(std::sync::Arc<S56<Span>>), | ||
| S57(std::sync::Arc<S57<Span>>), | ||
| S58(std::sync::Arc<S58<Span>>), | ||
| S59(std::sync::Arc<S59<Span>>), | ||
| } | ||
| } | ||
|
|
||
| mod expr { | ||
| pub struct E00<Span>(pub Expression<Span>); | ||
| pub struct E01<Span>(pub Expression<Span>); | ||
| pub struct E02<Span>(pub Expression<Span>); | ||
| pub struct E03<Span>(pub Expression<Span>); | ||
| pub struct E04<Span>(pub Expression<Span>); | ||
| pub struct E05<Span>(pub Expression<Span>); | ||
| pub struct E06<Span>(pub Expression<Span>); | ||
| pub struct E07<Span>(pub Expression<Span>); | ||
| pub struct E08<Span>(pub Expression<Span>); | ||
| pub struct E09<Span>(pub Expression<Span>); | ||
| pub struct E10<Span>(pub Expression<Span>); | ||
| pub struct E11<Span>(pub Expression<Span>); | ||
| pub struct E12<Span>(pub Expression<Span>); | ||
| pub struct E13<Span>(pub Expression<Span>); | ||
| pub struct E14<Span>(pub Expression<Span>); | ||
| pub struct E15<Span>(pub Expression<Span>); | ||
| pub struct E16<Span>(pub Expression<Span>); | ||
| pub struct E17<Span>(pub Expression<Span>); | ||
| pub struct E18<Span>(pub Expression<Span>); | ||
| pub struct E19<Span>(pub Expression<Span>); | ||
| pub struct E20<Span>(pub Expression<Span>); | ||
| pub struct E21<Span>(pub Expression<Span>); | ||
| pub struct E22<Span>(pub Expression<Span>); | ||
| pub struct E23<Span>(pub Expression<Span>); | ||
| pub struct E24<Span>(pub Expression<Span>); | ||
| pub struct E25<Span>(pub Expression<Span>); | ||
| pub struct E26<Span>(pub Expression<Span>); | ||
| pub struct E27<Span>(pub Expression<Span>); | ||
| pub struct E28<Span>(pub Expression<Span>); | ||
| pub struct E29<Span>(pub Expression<Span>); | ||
| pub struct E30<Span>(pub Expression<Span>); | ||
| pub struct E31<Span>(pub Expression<Span>); | ||
| pub struct E32<Span>(pub Expression<Span>); | ||
| pub struct E33<Span>(pub Expression<Span>); | ||
| pub struct E34<Span>(pub Expression<Span>); | ||
| pub struct E35<Span>(pub Expression<Span>); | ||
| pub struct E36<Span>(pub Expression<Span>); | ||
| pub struct E37<Span>(pub Expression<Span>); | ||
| pub struct E38<Span>(pub Expression<Span>); | ||
| pub struct E39<Span>(pub Expression<Span>); | ||
| pub struct E40<Span>(pub Expression<Span>); | ||
| pub struct E41<Span>(pub Expression<Span>); | ||
| pub struct E42<Span>(pub Expression<Span>); | ||
| pub struct E43<Span>(pub Expression<Span>); | ||
| pub struct E44<Span>(pub Expression<Span>); | ||
| pub struct E45<Span>(pub Expression<Span>); | ||
| pub struct E46<Span>(pub Expression<Span>); | ||
| pub struct E47<Span>(pub Expression<Span>); | ||
| pub struct E48<Span>(pub Expression<Span>); | ||
| pub struct E49<Span>(pub Expression<Span>); | ||
| pub struct E50<Span>(pub Expression<Span>); | ||
| pub struct E51<Span>(pub Expression<Span>); | ||
| pub struct E52<Span>(pub Expression<Span>); | ||
| pub struct E53<Span>(pub Expression<Span>); | ||
| pub struct E54<Span>(pub Expression<Span>); | ||
| pub struct E55<Span>(pub Expression<Span>); | ||
| pub struct E56<Span>(pub Expression<Span>); | ||
| pub struct E57<Span>(pub Expression<Span>); | ||
| pub struct E58<Span>(pub Expression<Span>); | ||
| pub struct E59<Span>(pub Expression<Span>); | ||
|
|
||
| pub enum Expression<Span> { | ||
| Class(std::sync::Arc<crate::Class<Span>>), | ||
| E00(std::sync::Arc<E00<Span>>), | ||
| E01(std::sync::Arc<E01<Span>>), | ||
| E02(std::sync::Arc<E02<Span>>), | ||
| E03(std::sync::Arc<E03<Span>>), | ||
| E04(std::sync::Arc<E04<Span>>), | ||
| E05(std::sync::Arc<E05<Span>>), | ||
| E06(std::sync::Arc<E06<Span>>), | ||
| E07(std::sync::Arc<E07<Span>>), | ||
| E08(std::sync::Arc<E08<Span>>), | ||
| E09(std::sync::Arc<E09<Span>>), | ||
| E10(std::sync::Arc<E10<Span>>), | ||
| E11(std::sync::Arc<E11<Span>>), | ||
| E12(std::sync::Arc<E12<Span>>), | ||
| E13(std::sync::Arc<E13<Span>>), | ||
| E14(std::sync::Arc<E14<Span>>), | ||
| E15(std::sync::Arc<E15<Span>>), | ||
| E16(std::sync::Arc<E16<Span>>), | ||
| E17(std::sync::Arc<E17<Span>>), | ||
| E18(std::sync::Arc<E18<Span>>), | ||
| E19(std::sync::Arc<E19<Span>>), | ||
| E20(std::sync::Arc<E20<Span>>), | ||
| E21(std::sync::Arc<E21<Span>>), | ||
| E22(std::sync::Arc<E22<Span>>), | ||
| E23(std::sync::Arc<E23<Span>>), | ||
| E24(std::sync::Arc<E24<Span>>), | ||
| E25(std::sync::Arc<E25<Span>>), | ||
| E26(std::sync::Arc<E26<Span>>), | ||
| E27(std::sync::Arc<E27<Span>>), | ||
| E28(std::sync::Arc<E28<Span>>), | ||
| E29(std::sync::Arc<E29<Span>>), | ||
| E30(std::sync::Arc<E30<Span>>), | ||
| E31(std::sync::Arc<E31<Span>>), | ||
| E32(std::sync::Arc<E32<Span>>), | ||
| E33(std::sync::Arc<E33<Span>>), | ||
| E34(std::sync::Arc<E34<Span>>), | ||
| E35(std::sync::Arc<E35<Span>>), | ||
| E36(std::sync::Arc<E36<Span>>), | ||
| E37(std::sync::Arc<E37<Span>>), | ||
| E38(std::sync::Arc<E38<Span>>), | ||
| E39(std::sync::Arc<E39<Span>>), | ||
| E40(std::sync::Arc<E40<Span>>), | ||
| E41(std::sync::Arc<E41<Span>>), | ||
| E42(std::sync::Arc<E42<Span>>), | ||
| E43(std::sync::Arc<E43<Span>>), | ||
| E44(std::sync::Arc<E44<Span>>), | ||
| E45(std::sync::Arc<E45<Span>>), | ||
| E46(std::sync::Arc<E46<Span>>), | ||
| E47(std::sync::Arc<E47<Span>>), | ||
| E48(std::sync::Arc<E48<Span>>), | ||
| E49(std::sync::Arc<E49<Span>>), | ||
| E50(std::sync::Arc<E50<Span>>), | ||
| E51(std::sync::Arc<E51<Span>>), | ||
| E52(std::sync::Arc<E52<Span>>), | ||
| E53(std::sync::Arc<E53<Span>>), | ||
| E54(std::sync::Arc<E54<Span>>), | ||
| E55(std::sync::Arc<E55<Span>>), | ||
| E56(std::sync::Arc<E56<Span>>), | ||
| E57(std::sync::Arc<E57<Span>>), | ||
| E58(std::sync::Arc<E58<Span>>), | ||
| E59(std::sync::Arc<E59<Span>>), | ||
| } | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That seems iffy to me as it can easily render the test flaky. It could lead to the test failing on slow machines or in CI if the OS is under heavy workload and suspends the process for a while. |
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious, does this have any effect on issue #114891?
View changes since the review