Auto merge of #101615 - compiler-errors:rpitit-perf, r=oli-obk

Make `compare_predicate_entailment` no longer a query

Make `compare_predicate_entailment` so it's no longer a query (again), and splits out the new logic (that equates the return types to infer RPITITs) into its own query. This means that this new query (now called `collect_trait_impl_trait_tys`) is no longer executed for non-RPITIT cases.

This should improve perf (https://github.com/rust-lang/rust/pull/101224#issuecomment-1241682203), though in practice we see that these some crates remain from the primary regressions list on the original report... They are all <= 0.43% regression and seemingly only on the incr-full scenario for all of them.

I am at a loss for what might be causing this regression other than what I fixed here, since we don't introduce much new non-RPITIT logic except for some `def_kind` query calls in some places, for example, like projection. Maybe that's it?

----

Originally this PR was opened to test enabling `cache_on_disk` (62164aaaa11) but that didn't turn out to be very useful (https://github.com/rust-lang/rust/pull/101615#issuecomment-1242403205), so that led me to just split the query (and rename the PR).
This commit is contained in:
bors 2022-09-13 15:33:06 +00:00
commit 1ce51982b8
7 changed files with 137 additions and 33 deletions

View File

@ -1755,7 +1755,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// In some (most?) cases cause.body_id points to actual body, but in some cases // In some (most?) cases cause.body_id points to actual body, but in some cases
// it's an actual definition. According to the comments (e.g. in // it's an actual definition. According to the comments (e.g. in
// librustc_typeck/check/compare_method.rs:compare_predicates_and_trait_impl_trait_tys) the latter // librustc_typeck/check/compare_method.rs:compare_predicate_entailment) the latter
// is relied upon by some other code. This might (or might not) need cleanup. // is relied upon by some other code. This might (or might not) need cleanup.
let body_owner_def_id = let body_owner_def_id =
self.tcx.hir().opt_local_def_id(cause.body_id).unwrap_or_else(|| { self.tcx.hir().opt_local_def_id(cause.body_id).unwrap_or_else(|| {

View File

@ -161,7 +161,7 @@ rustc_queries! {
separate_provide_extern separate_provide_extern
} }
query compare_predicates_and_trait_impl_trait_tys(key: DefId) query collect_trait_impl_trait_tys(key: DefId)
-> Result<&'tcx FxHashMap<DefId, Ty<'tcx>>, ErrorGuaranteed> -> Result<&'tcx FxHashMap<DefId, Ty<'tcx>>, ErrorGuaranteed>
{ {
desc { "better description please" } desc { "better description please" }

View File

@ -655,7 +655,7 @@ impl<'tcx> TyCtxt<'tcx> {
self, self,
def_id: DefId, def_id: DefId,
) -> ty::EarlyBinder<Result<&'tcx FxHashMap<DefId, Ty<'tcx>>, ErrorGuaranteed>> { ) -> ty::EarlyBinder<Result<&'tcx FxHashMap<DefId, Ty<'tcx>>, ErrorGuaranteed>> {
ty::EarlyBinder(self.compare_predicates_and_trait_impl_trait_tys(def_id)) ty::EarlyBinder(self.collect_trait_impl_trait_tys(def_id))
} }
pub fn bound_fn_sig(self, def_id: DefId) -> ty::EarlyBinder<ty::PolyFnSig<'tcx>> { pub fn bound_fn_sig(self, def_id: DefId) -> ty::EarlyBinder<ty::PolyFnSig<'tcx>> {

View File

@ -15,7 +15,7 @@ use rustc_middle::ty::error::{ExpectedFound, TypeError};
use rustc_middle::ty::subst::{InternalSubsts, Subst}; use rustc_middle::ty::subst::{InternalSubsts, Subst};
use rustc_middle::ty::util::ExplicitSelf; use rustc_middle::ty::util::ExplicitSelf;
use rustc_middle::ty::{ use rustc_middle::ty::{
self, DefIdTree, Ty, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitable, self, AssocItem, DefIdTree, Ty, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitable,
}; };
use rustc_middle::ty::{GenericParamDefKind, ToPredicate, TyCtxt}; use rustc_middle::ty::{GenericParamDefKind, ToPredicate, TyCtxt};
use rustc_span::Span; use rustc_span::Span;
@ -68,7 +68,10 @@ pub(crate) fn compare_impl_method<'tcx>(
return; return;
} }
tcx.ensure().compare_predicates_and_trait_impl_trait_tys(impl_m.def_id); if let Err(_) = compare_predicate_entailment(tcx, impl_m, impl_m_span, trait_m, impl_trait_ref)
{
return;
}
} }
/// This function is best explained by example. Consider a trait: /// This function is best explained by example. Consider a trait:
@ -137,15 +140,13 @@ pub(crate) fn compare_impl_method<'tcx>(
/// ///
/// Finally we register each of these predicates as an obligation and check that /// Finally we register each of these predicates as an obligation and check that
/// they hold. /// they hold.
pub(super) fn compare_predicates_and_trait_impl_trait_tys<'tcx>( fn compare_predicate_entailment<'tcx>(
tcx: TyCtxt<'tcx>, tcx: TyCtxt<'tcx>,
def_id: DefId, impl_m: &AssocItem,
) -> Result<&'tcx FxHashMap<DefId, Ty<'tcx>>, ErrorGuaranteed> { impl_m_span: Span,
let impl_m = tcx.opt_associated_item(def_id).unwrap(); trait_m: &AssocItem,
let impl_m_span = tcx.def_span(def_id); impl_trait_ref: ty::TraitRef<'tcx>,
let trait_m = tcx.opt_associated_item(impl_m.trait_item_def_id.unwrap()).unwrap(); ) -> Result<(), ErrorGuaranteed> {
let impl_trait_ref = tcx.impl_trait_ref(impl_m.impl_container(tcx).unwrap()).unwrap();
let trait_to_impl_substs = impl_trait_ref.substs; let trait_to_impl_substs = impl_trait_ref.substs;
// This node-id should be used for the `body_id` field on each // This node-id should be used for the `body_id` field on each
@ -164,7 +165,6 @@ pub(super) fn compare_predicates_and_trait_impl_trait_tys<'tcx>(
kind: impl_m.kind, kind: impl_m.kind,
}, },
); );
let return_span = tcx.hir().fn_decl_by_hir_id(impl_m_hir_id).unwrap().output.span();
// Create mapping from impl to placeholder. // Create mapping from impl to placeholder.
let impl_to_placeholder_substs = InternalSubsts::identity_for_item(tcx, impl_m.def_id); let impl_to_placeholder_substs = InternalSubsts::identity_for_item(tcx, impl_m.def_id);
@ -270,12 +270,6 @@ pub(super) fn compare_predicates_and_trait_impl_trait_tys<'tcx>(
let trait_sig = tcx.bound_fn_sig(trait_m.def_id).subst(tcx, trait_to_placeholder_substs); let trait_sig = tcx.bound_fn_sig(trait_m.def_id).subst(tcx, trait_to_placeholder_substs);
let trait_sig = tcx.liberate_late_bound_regions(impl_m.def_id, trait_sig); let trait_sig = tcx.liberate_late_bound_regions(impl_m.def_id, trait_sig);
let mut collector =
ImplTraitInTraitCollector::new(&ocx, return_span, param_env, impl_m_hir_id);
// FIXME(RPITIT): This should only be needed on the output type, but
// RPITIT placeholders shouldn't show up anywhere except for there,
// so I think this is fine.
let trait_sig = trait_sig.fold_with(&mut collector);
// Next, add all inputs and output as well-formed tys. Importantly, // Next, add all inputs and output as well-formed tys. Importantly,
// we have to do this before normalization, since the normalized ty may // we have to do this before normalization, since the normalized ty may
@ -436,6 +430,121 @@ pub(super) fn compare_predicates_and_trait_impl_trait_tys<'tcx>(
&outlives_environment, &outlives_environment,
); );
Ok(())
})
}
pub fn collect_trait_impl_trait_tys<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: DefId,
) -> Result<&'tcx FxHashMap<DefId, Ty<'tcx>>, ErrorGuaranteed> {
let impl_m = tcx.opt_associated_item(def_id).unwrap();
let trait_m = tcx.opt_associated_item(impl_m.trait_item_def_id.unwrap()).unwrap();
let impl_trait_ref = tcx.impl_trait_ref(impl_m.impl_container(tcx).unwrap()).unwrap();
let param_env = tcx.param_env(def_id);
let trait_to_impl_substs = impl_trait_ref.substs;
let impl_m_hir_id = tcx.hir().local_def_id_to_hir_id(impl_m.def_id.expect_local());
let return_span = tcx.hir().fn_decl_by_hir_id(impl_m_hir_id).unwrap().output.span();
let cause = ObligationCause::new(
return_span,
impl_m_hir_id,
ObligationCauseCode::CompareImplItemObligation {
impl_item_def_id: impl_m.def_id.expect_local(),
trait_item_def_id: trait_m.def_id,
kind: impl_m.kind,
},
);
// Create mapping from impl to placeholder.
let impl_to_placeholder_substs = InternalSubsts::identity_for_item(tcx, impl_m.def_id);
// Create mapping from trait to placeholder.
let trait_to_placeholder_substs =
impl_to_placeholder_substs.rebase_onto(tcx, impl_m.container_id(tcx), trait_to_impl_substs);
tcx.infer_ctxt().enter(|ref infcx| {
let ocx = ObligationCtxt::new(infcx);
let norm_cause = ObligationCause::misc(return_span, impl_m_hir_id);
let impl_return_ty = ocx.normalize(
norm_cause.clone(),
param_env,
infcx
.replace_bound_vars_with_fresh_vars(
return_span,
infer::HigherRankedType,
tcx.fn_sig(impl_m.def_id),
)
.output(),
);
let mut collector =
ImplTraitInTraitCollector::new(&ocx, return_span, param_env, impl_m_hir_id);
let unnormalized_trait_return_ty = tcx
.liberate_late_bound_regions(
impl_m.def_id,
tcx.bound_fn_sig(trait_m.def_id).subst(tcx, trait_to_placeholder_substs),
)
.output()
.fold_with(&mut collector);
let trait_return_ty =
ocx.normalize(norm_cause.clone(), param_env, unnormalized_trait_return_ty);
let wf_tys = FxHashSet::from_iter([unnormalized_trait_return_ty, trait_return_ty]);
match infcx.at(&cause, param_env).eq(trait_return_ty, impl_return_ty) {
Ok(infer::InferOk { value: (), obligations }) => {
ocx.register_obligations(obligations);
}
Err(terr) => {
let mut diag = struct_span_err!(
tcx.sess,
cause.span(),
E0053,
"method `{}` has an incompatible return type for trait",
trait_m.name
);
let hir = tcx.hir();
infcx.note_type_err(
&mut diag,
&cause,
hir.get_if_local(impl_m.def_id)
.and_then(|node| node.fn_decl())
.map(|decl| (decl.output.span(), "return type in trait".to_owned())),
Some(infer::ValuePairs::Terms(ExpectedFound {
expected: trait_return_ty.into(),
found: impl_return_ty.into(),
})),
terr,
false,
false,
);
return Err(diag.emit());
}
}
// Check that all obligations are satisfied by the implementation's
// RPITs.
let errors = ocx.select_all_or_error();
if !errors.is_empty() {
let reported = infcx.report_fulfillment_errors(&errors, None, false);
return Err(reported);
}
// Finally, resolve all regions. This catches wily misuses of
// lifetime parameters.
let outlives_environment = OutlivesEnvironment::with_bounds(
param_env,
Some(infcx),
infcx.implied_bounds_tys(param_env, impl_m_hir_id, wf_tys),
);
infcx.check_region_obligations_and_report_errors(
impl_m.def_id.expect_local(),
&outlives_environment,
);
let mut collected_tys = FxHashMap::default(); let mut collected_tys = FxHashMap::default();
for (def_id, (ty, substs)) in collector.types { for (def_id, (ty, substs)) in collector.types {
match infcx.fully_resolve(ty) { match infcx.fully_resolve(ty) {
@ -1322,7 +1431,7 @@ pub(crate) fn compare_ty_impl<'tcx>(
})(); })();
} }
/// The equivalent of [compare_predicates_and_trait_impl_trait_tys], but for associated types /// The equivalent of [compare_predicate_entailment], but for associated types
/// instead of associated functions. /// instead of associated functions.
fn compare_type_predicate_entailment<'tcx>( fn compare_type_predicate_entailment<'tcx>(
tcx: TyCtxt<'tcx>, tcx: TyCtxt<'tcx>,

View File

@ -132,7 +132,7 @@ use crate::require_c_abi_if_c_variadic;
use crate::util::common::indenter; use crate::util::common::indenter;
use self::coercion::DynamicCoerceMany; use self::coercion::DynamicCoerceMany;
use self::compare_method::compare_predicates_and_trait_impl_trait_tys; use self::compare_method::collect_trait_impl_trait_tys;
use self::region::region_scope_tree; use self::region::region_scope_tree;
pub use self::Expectation::*; pub use self::Expectation::*;
@ -250,7 +250,7 @@ pub fn provide(providers: &mut Providers) {
used_trait_imports, used_trait_imports,
check_mod_item_types, check_mod_item_types,
region_scope_tree, region_scope_tree,
compare_predicates_and_trait_impl_trait_tys, collect_trait_impl_trait_tys,
..*providers ..*providers
}; };
} }

View File

@ -9,7 +9,7 @@ trait Foo {
impl Foo for () { impl Foo for () {
fn bar() -> i32 { 0 } fn bar() -> i32 { 0 }
//~^ ERROR method `bar` has an incompatible type for trait //~^ ERROR method `bar` has an incompatible return type for trait
} }
fn main() {} fn main() {}

View File

@ -1,19 +1,14 @@
error[E0053]: method `bar` has an incompatible type for trait error[E0053]: method `bar` has an incompatible return type for trait
--> $DIR/deep-match.rs:11:17 --> $DIR/deep-match.rs:11:17
| |
LL | fn bar() -> i32 { 0 } LL | fn bar() -> i32 { 0 }
| ^^^ | ^^^
| | | |
| expected struct `Wrapper`, found `i32` | expected struct `Wrapper`, found `i32`
| help: change the output type to match the trait: `Wrapper<_>` | return type in trait
| |
note: type in trait = note: expected struct `Wrapper<_>`
--> $DIR/deep-match.rs:7:17 found type `i32`
|
LL | fn bar() -> Wrapper<impl Sized>;
| ^^^^^^^^^^^^^^^^^^^
= note: expected fn pointer `fn() -> Wrapper<_>`
found fn pointer `fn() -> i32`
error: aborting due to previous error error: aborting due to previous error