about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-09-13 15:33:06 +0000
committerbors <bors@rust-lang.org>2022-09-13 15:33:06 +0000
commit1ce51982b8550c782ded466c1abff0d2b2e21c4e (patch)
tree597e3ad33ce315008a6a4579da540eb46ef39ab7
parent5338f5f1d4ad9c99706e5cb691f8d5bbac341262 (diff)
parentbec079d1a9fe1a9163a0143ff9e1281b4eb09ec9 (diff)
downloadrust-1ce51982b8550c782ded466c1abff0d2b2e21c4e.tar.gz
rust-1ce51982b8550c782ded466c1abff0d2b2e21c4e.zip
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).
-rw-r--r--compiler/rustc_infer/src/infer/error_reporting/mod.rs2
-rw-r--r--compiler/rustc_middle/src/query/mod.rs2
-rw-r--r--compiler/rustc_middle/src/ty/util.rs2
-rw-r--r--compiler/rustc_typeck/src/check/compare_method.rs145
-rw-r--r--compiler/rustc_typeck/src/check/mod.rs4
-rw-r--r--src/test/ui/impl-trait/in-trait/deep-match.rs2
-rw-r--r--src/test/ui/impl-trait/in-trait/deep-match.stderr13
7 files changed, 137 insertions, 33 deletions
diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs
index 9d5a128f8de..67526c22289 100644
--- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs
+++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs
@@ -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(|| {
diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
index f72e7389fc6..aaceec98b1a 100644
--- a/compiler/rustc_middle/src/query/mod.rs
+++ b/compiler/rustc_middle/src/query/mod.rs
@@ -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" }
diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs
index 0c73ae54bc3..23ad4d27d8e 100644
--- a/compiler/rustc_middle/src/ty/util.rs
+++ b/compiler/rustc_middle/src/ty/util.rs
@@ -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>> {
diff --git a/compiler/rustc_typeck/src/check/compare_method.rs b/compiler/rustc_typeck/src/check/compare_method.rs
index 13a96df77b6..0c6d8f26f1d 100644
--- a/compiler/rustc_typeck/src/check/compare_method.rs
+++ b/compiler/rustc_typeck/src/check/compare_method.rs
@@ -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;
@@ -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:
@@ -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
@@ -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);
@@ -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
@@ -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) {
@@ -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>,
diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs
index 8811b38fc55..cfae63e4a06 100644
--- a/compiler/rustc_typeck/src/check/mod.rs
+++ b/compiler/rustc_typeck/src/check/mod.rs
@@ -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::*;
 
@@ -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
     };
 }
diff --git a/src/test/ui/impl-trait/in-trait/deep-match.rs b/src/test/ui/impl-trait/in-trait/deep-match.rs
index 5a220bc3f19..a6385147c3a 100644
--- a/src/test/ui/impl-trait/in-trait/deep-match.rs
+++ b/src/test/ui/impl-trait/in-trait/deep-match.rs
@@ -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() {}
diff --git a/src/test/ui/impl-trait/in-trait/deep-match.stderr b/src/test/ui/impl-trait/in-trait/deep-match.stderr
index af449869cb3..034ee5ea4e1 100644
--- a/src/test/ui/impl-trait/in-trait/deep-match.stderr
+++ b/src/test/ui/impl-trait/in-trait/deep-match.stderr
@@ -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