Auto merge of rust-lang#106129 - compiler-errors:compare_method-tweaks, r=BoxyUwU

Some `compare_method` tweaks

1. Make some of the comparison functions' names more regular
2. Reduce pub scope of some of the things in `compare_method`
~3. Remove some unnecessary opaque type handling code -- `InferCtxt` already is in a mode that doesn't define opaque types~
  * moved to a different PR
4. Bubble up `ErrorGuaranteed` for region constraint errors in `compare_method` - Improves a redundant error message in one unit test.
5. Move the `compare_method` module to have a more general name, since it's more like `compare_impl_item` :)
6. Rename `collect_trait_impl_trait_tys`

Author: bors

compiler/rustc_hir_analysis/src/check/check.rs

@@ -1,8 +1,8 @@
 use crate::check::intrinsicck::InlineAsmCtxt;
 use crate::errors::LinkageType;
 
-use super::compare_method::check_type_bounds;
-use super::compare_method::{compare_impl_method, compare_ty_impl};
+use super::compare_impl_item::check_type_bounds;
+use super::compare_impl_item::{compare_impl_method, compare_impl_ty};
 use super::*;
 use rustc_attr as attr;
 use rustc_errors::{Applicability, ErrorGuaranteed, MultiSpan};
@@ -468,7 +468,7 @@ fn check_opaque_meets_bounds<'tcx>(
         // Can have different predicates to their defining use
         hir::OpaqueTyOrigin::TyAlias => {
             let outlives_environment = OutlivesEnvironment::new(param_env);
-            infcx.check_region_obligations_and_report_errors(
+            let _ = infcx.check_region_obligations_and_report_errors(
                 defining_use_anchor,
                 &outlives_environment,
             );
@@ -774,7 +774,7 @@ fn check_impl_items_against_trait<'tcx>(
         let impl_item_full = tcx.hir().impl_item(impl_item.id);
         match impl_item_full.kind {
             hir::ImplItemKind::Const(..) => {
-                let _ = tcx.compare_assoc_const_impl_item_with_trait_item((
+                let _ = tcx.compare_impl_const((
                     impl_item.id.owner_id.def_id,
                     ty_impl_item.trait_item_def_id.unwrap(),
                 ));
@@ -791,7 +791,7 @@ fn check_impl_items_against_trait<'tcx>(
             }
             hir::ImplItemKind::Type(impl_ty) => {
                 let opt_trait_span = tcx.hir().span_if_local(ty_trait_item.def_id);
-                compare_ty_impl(
+                compare_impl_ty(
                     tcx,
                     &ty_impl_item,
                     impl_ty.span,

compiler/rustc_hir_analysis/src/check/compare_method.rs renamed to compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

@@ -34,7 +34,7 @@ use std::iter;
 /// - `impl_m_span`: span to use for reporting errors
 /// - `trait_m`: the method in the trait
 /// - `impl_trait_ref`: the TraitRef corresponding to the trait implementation
-pub(crate) fn compare_impl_method<'tcx>(
+pub(super) fn compare_impl_method<'tcx>(
     tcx: TyCtxt<'tcx>,
     impl_m: &ty::AssocItem,
     trait_m: &ty::AssocItem,
@@ -71,7 +71,7 @@ pub(crate) fn compare_impl_method<'tcx>(
         return;
     }
 
-    if let Err(_) = compare_predicate_entailment(
+    if let Err(_) = compare_method_predicate_entailment(
         tcx,
         impl_m,
         impl_m_span,
@@ -150,7 +150,7 @@ pub(crate) fn compare_impl_method<'tcx>(
 /// Finally we register each of these predicates as an obligation and check that
 /// they hold.
 #[instrument(level = "debug", skip(tcx, impl_m_span, impl_trait_ref))]
-fn compare_predicate_entailment<'tcx>(
+fn compare_method_predicate_entailment<'tcx>(
     tcx: TyCtxt<'tcx>,
     impl_m: &ty::AssocItem,
     impl_m_span: Span,
@@ -337,7 +337,7 @@ fn compare_predicate_entailment<'tcx>(
     if !errors.is_empty() {
         match check_implied_wf {
             CheckImpliedWfMode::Check => {
-                return compare_predicate_entailment(
+                return compare_method_predicate_entailment(
                     tcx,
                     impl_m,
                     impl_m_span,
@@ -374,7 +374,7 @@ fn compare_predicate_entailment<'tcx>(
         // becomes a hard error (i.e. ideally we'd just call `resolve_regions_and_report_errors`
         match check_implied_wf {
             CheckImpliedWfMode::Check => {
-                return compare_predicate_entailment(
+                return compare_method_predicate_entailment(
                     tcx,
                     impl_m,
                     impl_m_span,
@@ -407,7 +407,7 @@ enum CheckImpliedWfMode {
     /// re-check with `Skip`, and emit a lint if it succeeds.
     Check,
     /// Skips checking implied well-formedness of the impl method, but will emit
-    /// a lint if the `compare_predicate_entailment` succeeded. This means that
+    /// a lint if the `compare_method_predicate_entailment` succeeded. This means that
     /// the reason that we had failed earlier during `Check` was due to the impl
     /// having stronger requirements than the trait.
     Skip,
@@ -441,8 +441,41 @@ fn compare_asyncness<'tcx>(
     Ok(())
 }
 
+/// Given a method def-id in an impl, compare the method signature of the impl
+/// against the trait that it's implementing. In doing so, infer the hidden types
+/// that this method's signature provides to satisfy each return-position `impl Trait`
+/// in the trait signature.
+///
+/// The method is also responsible for making sure that the hidden types for each
+/// RPITIT actually satisfy the bounds of the `impl Trait`, i.e. that if we infer
+/// `impl Trait = Foo`, that `Foo: Trait` holds.
+///
+/// For example, given the sample code:
+///
+/// ```
+/// #![feature(return_position_impl_trait_in_trait)]
+///
+/// use std::ops::Deref;
+///
+/// trait Foo {
+///     fn bar() -> impl Deref<Target = impl Sized>;
+///              // ^- RPITIT #1        ^- RPITIT #2
+/// }
+///
+/// impl Foo for () {
+///     fn bar() -> Box<String> { Box::new(String::new()) }
+/// }
+/// ```
+///
+/// The hidden types for the RPITITs in `bar` would be inferred to:
+///     * `impl Deref` (RPITIT #1) = `Box<String>`
+///     * `impl Sized` (RPITIT #2) = `String`
+///
+/// The relationship between these two types is straightforward in this case, but
+/// may be more tenuously connected via other `impl`s and normalization rules for
+/// cases of more complicated nested RPITITs.
 #[instrument(skip(tcx), level = "debug", ret)]
-pub fn collect_trait_impl_trait_tys<'tcx>(
+pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
     tcx: TyCtxt<'tcx>,
     def_id: DefId,
 ) -> Result<&'tcx FxHashMap<DefId, Ty<'tcx>>, ErrorGuaranteed> {
@@ -550,13 +583,13 @@ pub fn collect_trait_impl_trait_tys<'tcx>(
     // Unify the whole function signature. We need to do this to fully infer
     // the lifetimes of the return type, but do this after unifying just the
     // return types, since we want to avoid duplicating errors from
-    // `compare_predicate_entailment`.
+    // `compare_method_predicate_entailment`.
     match ocx.eq(&cause, param_env, trait_fty, impl_fty) {
         Ok(()) => {}
         Err(terr) => {
-            // This function gets called during `compare_predicate_entailment` when normalizing a
+            // This function gets called during `compare_method_predicate_entailment` when normalizing a
             // signature that contains RPITIT. When the method signatures don't match, we have to
-            // emit an error now because `compare_predicate_entailment` will not report the error
+            // emit an error now because `compare_method_predicate_entailment` will not report the error
             // when normalization fails.
             let emitted = report_trait_method_mismatch(
                 infcx,
@@ -589,7 +622,7 @@ pub fn collect_trait_impl_trait_tys<'tcx>(
     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 {
@@ -1516,8 +1549,8 @@ fn compare_generic_param_kinds<'tcx>(
     Ok(())
 }
 
-/// Use `tcx.compare_assoc_const_impl_item_with_trait_item` instead
-pub(crate) fn raw_compare_const_impl(
+/// Use `tcx.compare_impl_const` instead
+pub(super) fn compare_impl_const_raw(
     tcx: TyCtxt<'_>,
     (impl_const_item_def, trait_const_item_def): (LocalDefId, DefId),
 ) -> Result<(), ErrorGuaranteed> {
@@ -1617,13 +1650,13 @@ pub(crate) fn raw_compare_const_impl(
         return Err(infcx.err_ctxt().report_fulfillment_errors(&errors, None));
     }
 
-    // FIXME return `ErrorReported` if region obligations error?
     let outlives_environment = OutlivesEnvironment::new(param_env);
-    infcx.check_region_obligations_and_report_errors(impl_const_item_def, &outlives_environment);
+    infcx.check_region_obligations_and_report_errors(impl_const_item_def, &outlives_environment)?;
+
     Ok(())
 }
 
-pub(crate) fn compare_ty_impl<'tcx>(
+pub(super) fn compare_impl_ty<'tcx>(
     tcx: TyCtxt<'tcx>,
     impl_ty: &ty::AssocItem,
     impl_ty_span: Span,
@@ -1645,7 +1678,7 @@ pub(crate) fn compare_ty_impl<'tcx>(
     })();
 }
 
-/// The equivalent of [compare_predicate_entailment], but for associated types
+/// The equivalent of [compare_method_predicate_entailment], but for associated types
 /// instead of associated functions.
 fn compare_type_predicate_entailment<'tcx>(
     tcx: TyCtxt<'tcx>,
@@ -1730,7 +1763,7 @@ fn compare_type_predicate_entailment<'tcx>(
     infcx.check_region_obligations_and_report_errors(
         impl_ty.def_id.expect_local(),
         &outlives_environment,
-    );
+    )?;
 
     Ok(())
 }
@@ -1749,7 +1782,7 @@ fn compare_type_predicate_entailment<'tcx>(
 /// from the impl could be overridden). We also can't normalize generic
 /// associated types (yet) because they contain bound parameters.
 #[instrument(level = "debug", skip(tcx))]
-pub fn check_type_bounds<'tcx>(
+pub(super) fn check_type_bounds<'tcx>(
     tcx: TyCtxt<'tcx>,
     trait_ty: &ty::AssocItem,
     impl_ty: &ty::AssocItem,
@@ -1944,7 +1977,7 @@ pub fn check_type_bounds<'tcx>(
     infcx.check_region_obligations_and_report_errors(
         impl_ty.def_id.expect_local(),
         &outlives_environment,
-    );
+    )?;
 
     let constraints = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();
     for (key, value) in constraints {

compiler/rustc_hir_analysis/src/check/mod.rs

@@ -63,7 +63,7 @@ a type parameter).
 */
 
 mod check;
-mod compare_method;
+mod compare_impl_item;
 pub mod dropck;
 pub mod intrinsic;
 pub mod intrinsicck;
@@ -94,7 +94,7 @@ use std::num::NonZeroU32;
 use crate::require_c_abi_if_c_variadic;
 use crate::util::common::indenter;
 
-use self::compare_method::collect_trait_impl_trait_tys;
+use self::compare_impl_item::collect_return_position_impl_trait_in_trait_tys;
 use self::region::region_scope_tree;
 
 pub fn provide(providers: &mut Providers) {
@@ -103,8 +103,8 @@ pub fn provide(providers: &mut Providers) {
         adt_destructor,
         check_mod_item_types,
         region_scope_tree,
-        collect_trait_impl_trait_tys,
-        compare_assoc_const_impl_item_with_trait_item: compare_method::raw_compare_const_impl,
+        collect_return_position_impl_trait_in_trait_tys,
+        compare_impl_const: compare_impl_item::compare_impl_const_raw,
         ..*providers
     };
 }

compiler/rustc_hir_analysis/src/check/wfcheck.rs

@@ -115,7 +115,7 @@ pub(super) fn enter_wf_checking_ctxt<'tcx, F>(
     let outlives_environment =
         OutlivesEnvironment::with_bounds(param_env, Some(infcx), implied_bounds);
 
-    infcx.check_region_obligations_and_report_errors(body_def_id, &outlives_environment);
+    let _ = infcx.check_region_obligations_and_report_errors(body_def_id, &outlives_environment);
 }
 
 fn check_well_formed(tcx: TyCtxt<'_>, def_id: hir::OwnerId) {

compiler/rustc_hir_analysis/src/coherence/builtin.rs

@@ -325,7 +325,7 @@ fn visit_implementation_of_dispatch_from_dyn(tcx: TyCtxt<'_>, impl_did: LocalDef
 
                 // Finally, resolve all regions.
                 let outlives_env = OutlivesEnvironment::new(param_env);
-                infcx.check_region_obligations_and_report_errors(impl_did, &outlives_env);
+                let _ = infcx.check_region_obligations_and_report_errors(impl_did, &outlives_env);
             }
         }
         _ => {
@@ -565,7 +565,7 @@ pub fn coerce_unsized_info<'tcx>(tcx: TyCtxt<'tcx>, impl_did: DefId) -> CoerceUn
 
     // Finally, resolve all regions.
     let outlives_env = OutlivesEnvironment::new(param_env);
-    infcx.check_region_obligations_and_report_errors(impl_did, &outlives_env);
+    let _ = infcx.check_region_obligations_and_report_errors(impl_did, &outlives_env);
 
     CoerceUnsizedInfo { custom_kind: kind }
 }

compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs

@@ -181,7 +181,7 @@ fn get_impl_substs(
 
     let implied_bounds = infcx.implied_bounds_tys(param_env, impl1_hir_id, assumed_wf_types);
     let outlives_env = OutlivesEnvironment::with_bounds(param_env, Some(infcx), implied_bounds);
-    infcx.check_region_obligations_and_report_errors(impl1_def_id, &outlives_env);
+    let _ = infcx.check_region_obligations_and_report_errors(impl1_def_id, &outlives_env);
     let Ok(impl2_substs) = infcx.fully_resolve(impl2_substs) else {
         let span = tcx.def_span(impl1_def_id);
         tcx.sess.emit_err(SubstsOnOverriddenImpl { span });

compiler/rustc_infer/src/infer/error_reporting/mod.rs

@@ -1836,7 +1836,7 @@ impl<'tcx> TypeErrCtxt<'_, '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
-        // rustc_hir_analysis/check/compare_method.rs:compare_predicate_entailment) the latter
+        // rustc_hir_analysis/check/compare_impl_item.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(|| {

compiler/rustc_infer/src/infer/mod.rs

@@ -1693,7 +1693,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
         &self,
         generic_param_scope: LocalDefId,
         outlives_env: &OutlivesEnvironment<'tcx>,
-    ) -> Option<ErrorGuaranteed> {
+    ) -> Result<(), ErrorGuaranteed> {
         let errors = self.resolve_regions(outlives_env);
 
         if let None = self.tainted_by_errors() {
@@ -1705,9 +1705,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
             self.report_region_errors(generic_param_scope, &errors);
         }
 
-        (!errors.is_empty()).then(|| {
-            self.tcx.sess.delay_span_bug(rustc_span::DUMMY_SP, "error should have been emitted")
-        })
+        if errors.is_empty() {
+            Ok(())
+        } else {
+            Err(self
+                .tcx
+                .sess
+                .delay_span_bug(rustc_span::DUMMY_SP, "error should have been emitted"))
+        }
     }
 
     // [Note-Type-error-reporting]

compiler/rustc_infer/src/infer/outlives/obligations.rs

@@ -178,7 +178,7 @@ impl<'tcx> InferCtxt<'tcx> {
         &self,
         generic_param_scope: LocalDefId,
         outlives_env: &OutlivesEnvironment<'tcx>,
-    ) -> Option<ErrorGuaranteed> {
+    ) -> Result<(), ErrorGuaranteed> {
         self.process_registered_region_obligations(
             outlives_env.region_bound_pairs(),
             outlives_env.param_env,

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -223,7 +223,7 @@ provide! { tcx, def_id, other, cdata,
     generator_kind => { table }
     trait_def => { table }
     deduced_param_attrs => { table }
-    collect_trait_impl_trait_tys => {
+    collect_return_position_impl_trait_in_trait_tys => {
         Ok(cdata
             .root
             .tables