Skip to content

Commit

Permalink
Auto merge of rust-lang#101615 - compiler-errors:rpitit-perf, r=oli-obk
Browse files Browse the repository at this point in the history
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 (rust-lang#101224 (comment)), 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 (rust-lang#101615 (comment)), so that led me to just split the query (and rename the PR).
  • Loading branch information
bors committed Sep 13, 2022
2 parents 5338f5f + bec079d commit 1ce5198
Show file tree
Hide file tree
Showing 7 changed files with 137 additions and 33 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
// 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.
let body_owner_def_id =
self.tcx.hir().opt_local_def_id(cause.body_id).unwrap_or_else(|| {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ rustc_queries! {
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>
{
desc { "better description please" }
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,7 @@ impl<'tcx> TyCtxt<'tcx> {
self,
def_id: DefId,
) -> 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>> {
Expand Down
145 changes: 127 additions & 18 deletions compiler/rustc_typeck/src/check/compare_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_middle::ty::error::{ExpectedFound, TypeError};
use rustc_middle::ty::subst::{InternalSubsts, Subst};
use rustc_middle::ty::util::ExplicitSelf;
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_span::Span;
Expand Down Expand Up @@ -68,7 +68,10 @@ pub(crate) fn compare_impl_method<'tcx>(
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:
Expand Down Expand Up @@ -137,15 +140,13 @@ pub(crate) fn compare_impl_method<'tcx>(
///
/// Finally we register each of these predicates as an obligation and check that
/// they hold.
pub(super) fn compare_predicates_and_trait_impl_trait_tys<'tcx>(
fn compare_predicate_entailment<'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 impl_m_span = tcx.def_span(def_id);
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();

impl_m: &AssocItem,
impl_m_span: Span,
trait_m: &AssocItem,
impl_trait_ref: ty::TraitRef<'tcx>,
) -> Result<(), ErrorGuaranteed> {
let trait_to_impl_substs = impl_trait_ref.substs;

// This node-id should be used for the `body_id` field on each
Expand All @@ -164,7 +165,6 @@ pub(super) fn compare_predicates_and_trait_impl_trait_tys<'tcx>(
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.
let impl_to_placeholder_substs = InternalSubsts::identity_for_item(tcx, impl_m.def_id);
Expand Down Expand Up @@ -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.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,
// we have to do this before normalization, since the normalized ty may
Expand Down Expand Up @@ -436,6 +430,121 @@ pub(super) fn compare_predicates_and_trait_impl_trait_tys<'tcx>(
&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();
for (def_id, (ty, substs)) in collector.types {
match infcx.fully_resolve(ty) {
Expand Down Expand Up @@ -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.
fn compare_type_predicate_entailment<'tcx>(
tcx: TyCtxt<'tcx>,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_typeck/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ use crate::require_c_abi_if_c_variadic;
use crate::util::common::indenter;

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;
pub use self::Expectation::*;

Expand Down Expand Up @@ -250,7 +250,7 @@ pub fn provide(providers: &mut Providers) {
used_trait_imports,
check_mod_item_types,
region_scope_tree,
compare_predicates_and_trait_impl_trait_tys,
collect_trait_impl_trait_tys,
..*providers
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/impl-trait/in-trait/deep-match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ trait Foo {

impl Foo for () {
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() {}
13 changes: 4 additions & 9 deletions src/test/ui/impl-trait/in-trait/deep-match.stderr
Original file line number Diff line number Diff line change
@@ -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
|
LL | fn bar() -> i32 { 0 }
| ^^^
| |
| expected struct `Wrapper`, found `i32`
| help: change the output type to match the trait: `Wrapper<_>`
| return type in trait
|
note: type in trait
--> $DIR/deep-match.rs:7:17
|
LL | fn bar() -> Wrapper<impl Sized>;
| ^^^^^^^^^^^^^^^^^^^
= note: expected fn pointer `fn() -> Wrapper<_>`
found fn pointer `fn() -> i32`
= note: expected struct `Wrapper<_>`
found type `i32`

error: aborting due to previous error

Expand Down

0 comments on commit 1ce5198

Please sign in to comment.