Rollup merge of #110577 - compiler-errors:drop-impl-fulfill, r=lcnr

Use fulfillment to check `Drop` impl compatibility

Use an `ObligationCtxt` to ensure that a `Drop` impl does not have stricter requirements than the ADT that it's implemented for, rather than using a `SimpleEqRelation` to (more or less) syntactically equate predicates on an ADT with predicates on an impl.

r? types

### Some background

The old code reads:

```rust
// An earlier version of this code attempted to do this checking
// via the traits::fulfill machinery. However, it ran into trouble
// since the fulfill machinery merely turns outlives-predicates
// 'a:'b and T:'b into region inference constraints. It is simpler
// just to look for all the predicates directly.
```

I'm not sure what this means, but perhaps in the 8 years since that this comment was written (cc #23638) it's gotten easier to process region constraints after doing fulfillment? I don't know how this logic differs from anything we do in the `compare_impl_item` module. Ironically, later on it says:

```rust
// However, it may be more efficient in the future to batch
// the analysis together via the fulfill (see comment above regarding
// the usage of the fulfill machinery), rather than the
// repeated `.iter().any(..)` calls.
```

Also:
* Removes `SimpleEqRelation` which was far too syntactical in its relation.
* Fixes #110557

Author: matthiaskrgr

compiler/rustc_hir_analysis/src/check/dropck.rs

@@ -1,12 +1,14 @@
 // FIXME(@lcnr): Move this module out of `rustc_hir_analysis`.
 //
 // We don't do any drop checking during hir typeck.
+use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::{struct_span_err, ErrorGuaranteed};
-use rustc_middle::ty::error::TypeError;
-use rustc_middle::ty::relate::{Relate, RelateResult, TypeRelation};
+use rustc_infer::infer::outlives::env::OutlivesEnvironment;
+use rustc_infer::infer::{RegionResolutionError, TyCtxtInferExt};
 use rustc_middle::ty::subst::SubstsRef;
 use rustc_middle::ty::util::IgnoreRegions;
-use rustc_middle::ty::{self, Predicate, Ty, TyCtxt};
+use rustc_middle::ty::{self, TyCtxt};
+use rustc_trait_selection::traits::{self, ObligationCtxt};
 
 use crate::errors;
 use crate::hir::def_id::{DefId, LocalDefId};
@@ -43,21 +45,20 @@ pub fn check_drop_impl(tcx: TyCtxt<'_>, drop_impl_did: DefId) -> Result<(), Erro
         }
     }
     let dtor_self_type = tcx.type_of(drop_impl_did).subst_identity();
-    let dtor_predicates = tcx.predicates_of(drop_impl_did);
     match dtor_self_type.kind() {
-        ty::Adt(adt_def, self_to_impl_substs) => {
+        ty::Adt(adt_def, adt_to_impl_substs) => {
             ensure_drop_params_and_item_params_correspond(
                 tcx,
                 drop_impl_did.expect_local(),
                 adt_def.did(),
-                self_to_impl_substs,
+                adt_to_impl_substs,
             )?;
 
             ensure_drop_predicates_are_implied_by_item_defn(
                 tcx,
-                dtor_predicates,
+                drop_impl_did.expect_local(),
                 adt_def.did().expect_local(),
-                self_to_impl_substs,
+                adt_to_impl_substs,
             )
         }
         _ => {
@@ -78,9 +79,9 @@ fn ensure_drop_params_and_item_params_correspond<'tcx>(
     tcx: TyCtxt<'tcx>,
     drop_impl_did: LocalDefId,
     self_type_did: DefId,
-    drop_impl_substs: SubstsRef<'tcx>,
+    adt_to_impl_substs: SubstsRef<'tcx>,
 ) -> Result<(), ErrorGuaranteed> {
-    let Err(arg) = tcx.uses_unique_generic_params(drop_impl_substs, IgnoreRegions::No) else {
+    let Err(arg) = tcx.uses_unique_generic_params(adt_to_impl_substs, IgnoreRegions::No) else {
         return Ok(())
     };
 
@@ -111,237 +112,94 @@ fn ensure_drop_params_and_item_params_correspond<'tcx>(
 /// implied by assuming the predicates attached to self_type_did.
 fn ensure_drop_predicates_are_implied_by_item_defn<'tcx>(
     tcx: TyCtxt<'tcx>,
-    dtor_predicates: ty::GenericPredicates<'tcx>,
-    self_type_did: LocalDefId,
-    self_to_impl_substs: SubstsRef<'tcx>,
+    drop_impl_def_id: LocalDefId,
+    adt_def_id: LocalDefId,
+    adt_to_impl_substs: SubstsRef<'tcx>,
 ) -> Result<(), ErrorGuaranteed> {
-    let mut result = Ok(());
-
-    // Here is an example, analogous to that from
-    // `compare_impl_method`.
-    //
-    // Consider a struct type:
-    //
-    //     struct Type<'c, 'b:'c, 'a> {
-    //         x: &'a Contents            // (contents are irrelevant;
-    //         y: &'c Cell<&'b Contents>, //  only the bounds matter for our purposes.)
-    //     }
-    //
-    // and a Drop impl:
-    //
-    //     impl<'z, 'y:'z, 'x:'y> Drop for P<'z, 'y, 'x> {
-    //         fn drop(&mut self) { self.y.set(self.x); } // (only legal if 'x: 'y)
-    //     }
-    //
-    // We start out with self_to_impl_substs, that maps the generic
-    // parameters of Type to that of the Drop impl.
+    let infcx = tcx.infer_ctxt().build();
+    let ocx = ObligationCtxt::new(&infcx);
+
+    // Take the param-env of the adt and substitute the substs that show up in
+    // the implementation's self type. This gives us the assumptions that the
+    // self ty of the implementation is allowed to know just from it being a
+    // well-formed adt, since that's all we're allowed to assume while proving
+    // the Drop implementation is not specialized.
     //
-    //     self_to_impl_substs = {'c => 'z, 'b => 'y, 'a => 'x}
-    //
-    // Applying this to the predicates (i.e., assumptions) provided by the item
-    // definition yields the instantiated assumptions:
-    //
-    //     ['y : 'z]
-    //
-    // We then check all of the predicates of the Drop impl:
-    //
-    //     ['y:'z, 'x:'y]
-    //
-    // and ensure each is in the list of instantiated
-    // assumptions. Here, `'y:'z` is present, but `'x:'y` is
-    // absent. So we report an error that the Drop impl injected a
-    // predicate that is not present on the struct definition.
-
-    // We can assume the predicates attached to struct/enum definition
-    // hold.
-    let generic_assumptions = tcx.predicates_of(self_type_did);
-
-    let assumptions_in_impl_context = generic_assumptions.instantiate(tcx, &self_to_impl_substs);
-    let assumptions_in_impl_context = assumptions_in_impl_context.predicates;
-
-    debug!(?assumptions_in_impl_context, ?dtor_predicates.predicates);
-
-    let self_param_env = tcx.param_env(self_type_did);
-
-    // An earlier version of this code attempted to do this checking
-    // via the traits::fulfill machinery. However, it ran into trouble
-    // since the fulfill machinery merely turns outlives-predicates
-    // 'a:'b and T:'b into region inference constraints. It is simpler
-    // just to look for all the predicates directly.
-
-    assert_eq!(dtor_predicates.parent, None);
-    for &(predicate, predicate_sp) in dtor_predicates.predicates {
-        // (We do not need to worry about deep analysis of type
-        // expressions etc because the Drop impls are already forced
-        // to take on a structure that is roughly an alpha-renaming of
-        // the generic parameters of the item definition.)
-
-        // This path now just checks *all* predicates via an instantiation of
-        // the `SimpleEqRelation`, which simply forwards to the `relate` machinery
-        // after taking care of anonymizing late bound regions.
-        //
-        // However, it may be more efficient in the future to batch
-        // the analysis together via the fulfill (see comment above regarding
-        // the usage of the fulfill machinery), rather than the
-        // repeated `.iter().any(..)` calls.
+    // We don't need to normalize this param-env or anything, since we're only
+    // substituting it with free params, so no additional param-env normalization
+    // can occur on top of what has been done in the param_env query itself.
+    let param_env = ty::EarlyBinder(tcx.param_env(adt_def_id))
+        .subst(tcx, adt_to_impl_substs)
+        .with_constness(tcx.constness(drop_impl_def_id));
+
+    for (pred, span) in tcx.predicates_of(drop_impl_def_id).instantiate_identity(tcx) {
+        let normalize_cause = traits::ObligationCause::misc(span, adt_def_id);
+        let pred = ocx.normalize(&normalize_cause, param_env, pred);
+        let cause = traits::ObligationCause::new(span, adt_def_id, traits::DropImpl);
+        ocx.register_obligation(traits::Obligation::new(tcx, cause, param_env, pred));
+    }
 
-        // This closure is a more robust way to check `Predicate` equality
-        // than simple `==` checks (which were the previous implementation).
-        // It relies on `ty::relate` for `TraitPredicate`, `ProjectionPredicate`,
-        // `ConstEvaluatable` and `TypeOutlives` (which implement the Relate trait),
-        // while delegating on simple equality for the other `Predicate`.
-        // This implementation solves (Issue #59497) and (Issue #58311).
-        // It is unclear to me at the moment whether the approach based on `relate`
-        // could be extended easily also to the other `Predicate`.
-        let predicate_matches_closure = |p: Predicate<'tcx>| {
-            let mut relator: SimpleEqRelation<'tcx> = SimpleEqRelation::new(tcx, self_param_env);
-            let predicate = predicate.kind();
-            let p = p.kind();
-            match (predicate.skip_binder(), p.skip_binder()) {
-                (
-                    ty::PredicateKind::Clause(ty::Clause::Trait(a)),
-                    ty::PredicateKind::Clause(ty::Clause::Trait(b)),
-                ) => relator.relate(predicate.rebind(a), p.rebind(b)).is_ok(),
-                (
-                    ty::PredicateKind::Clause(ty::Clause::Projection(a)),
-                    ty::PredicateKind::Clause(ty::Clause::Projection(b)),
-                ) => relator.relate(predicate.rebind(a), p.rebind(b)).is_ok(),
-                (
-                    ty::PredicateKind::ConstEvaluatable(a),
-                    ty::PredicateKind::ConstEvaluatable(b),
-                ) => relator.relate(predicate.rebind(a), predicate.rebind(b)).is_ok(),
-                (
-                    ty::PredicateKind::Clause(ty::Clause::TypeOutlives(ty::OutlivesPredicate(
-                        ty_a,
-                        lt_a,
-                    ))),
-                    ty::PredicateKind::Clause(ty::Clause::TypeOutlives(ty::OutlivesPredicate(
-                        ty_b,
-                        lt_b,
-                    ))),
-                ) => {
-                    relator.relate(predicate.rebind(ty_a), p.rebind(ty_b)).is_ok()
-                        && relator.relate(predicate.rebind(lt_a), p.rebind(lt_b)).is_ok()
-                }
-                (ty::PredicateKind::WellFormed(arg_a), ty::PredicateKind::WellFormed(arg_b)) => {
-                    relator.relate(predicate.rebind(arg_a), p.rebind(arg_b)).is_ok()
-                }
-                _ => predicate == p,
+    // All of the custom error reporting logic is to preserve parity with the old
+    // error messages.
+    //
+    // They can probably get removed with better treatment of the new `DropImpl`
+    // obligation cause code, and perhaps some custom logic in `report_region_errors`.
+
+    let errors = ocx.select_all_or_error();
+    if !errors.is_empty() {
+        let mut guar = None;
+        let mut root_predicates = FxHashSet::default();
+        for error in errors {
+            let root_predicate = error.root_obligation.predicate;
+            if root_predicates.insert(root_predicate) {
+                let item_span = tcx.def_span(adt_def_id);
+                let self_descr = tcx.def_descr(adt_def_id.to_def_id());
+                guar = Some(
+                    struct_span_err!(
+                        tcx.sess,
+                        error.root_obligation.cause.span,
+                        E0367,
+                        "`Drop` impl requires `{root_predicate}` \
+                        but the {self_descr} it is implemented for does not",
+                    )
+                    .span_note(item_span, "the implementor must specify the same requirement")
+                    .emit(),
+                );
             }
-        };
-
-        if !assumptions_in_impl_context.iter().copied().any(predicate_matches_closure) {
-            let item_span = tcx.def_span(self_type_did);
-            let self_descr = tcx.def_descr(self_type_did.to_def_id());
-            let reported = struct_span_err!(
-                tcx.sess,
-                predicate_sp,
-                E0367,
-                "`Drop` impl requires `{predicate}` but the {self_descr} it is implemented for does not",
-            )
-            .span_note(item_span, "the implementor must specify the same requirement")
-            .emit();
-            result = Err(reported);
         }
+        return Err(guar.unwrap());
     }
 
-    result
-}
-
-/// This is an implementation of the [`TypeRelation`] trait with the
-/// aim of simply comparing for equality (without side-effects).
-///
-/// It is not intended to be used anywhere else other than here.
-pub(crate) struct SimpleEqRelation<'tcx> {
-    tcx: TyCtxt<'tcx>,
-    param_env: ty::ParamEnv<'tcx>,
-}
-
-impl<'tcx> SimpleEqRelation<'tcx> {
-    fn new(tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> SimpleEqRelation<'tcx> {
-        SimpleEqRelation { tcx, param_env }
-    }
-}
-
-impl<'tcx> TypeRelation<'tcx> for SimpleEqRelation<'tcx> {
-    fn tcx(&self) -> TyCtxt<'tcx> {
-        self.tcx
-    }
-
-    fn param_env(&self) -> ty::ParamEnv<'tcx> {
-        self.param_env
-    }
-
-    fn tag(&self) -> &'static str {
-        "dropck::SimpleEqRelation"
-    }
-
-    fn a_is_expected(&self) -> bool {
-        true
-    }
-
-    fn relate_with_variance<T: Relate<'tcx>>(
-        &mut self,
-        _: ty::Variance,
-        _info: ty::VarianceDiagInfo<'tcx>,
-        a: T,
-        b: T,
-    ) -> RelateResult<'tcx, T> {
-        // Here we ignore variance because we require drop impl's types
-        // to be *exactly* the same as to the ones in the struct definition.
-        self.relate(a, b)
-    }
-
-    fn tys(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> {
-        debug!("SimpleEqRelation::tys(a={:?}, b={:?})", a, b);
-        ty::relate::super_relate_tys(self, a, b)
-    }
-
-    fn regions(
-        &mut self,
-        a: ty::Region<'tcx>,
-        b: ty::Region<'tcx>,
-    ) -> RelateResult<'tcx, ty::Region<'tcx>> {
-        debug!("SimpleEqRelation::regions(a={:?}, b={:?})", a, b);
-
-        // We can just equate the regions because LBRs have been
-        // already anonymized.
-        if a == b {
-            Ok(a)
-        } else {
-            // I'm not sure is this `TypeError` is the right one, but
-            // it should not matter as it won't be checked (the dropck
-            // will emit its own, more informative and higher-level errors
-            // in case anything goes wrong).
-            Err(TypeError::RegionsPlaceholderMismatch)
+    let errors = ocx.infcx.resolve_regions(&OutlivesEnvironment::new(param_env));
+    if !errors.is_empty() {
+        let mut guar = None;
+        for error in errors {
+            let item_span = tcx.def_span(adt_def_id);
+            let self_descr = tcx.def_descr(adt_def_id.to_def_id());
+            let outlives = match error {
+                RegionResolutionError::ConcreteFailure(_, a, b) => format!("{b}: {a}"),
+                RegionResolutionError::GenericBoundFailure(_, generic, r) => {
+                    format!("{generic}: {r}")
+                }
+                RegionResolutionError::SubSupConflict(_, _, _, a, _, b, _) => format!("{b}: {a}"),
+                RegionResolutionError::UpperBoundUniverseConflict(a, _, _, _, b) => {
+                    format!("{b}: {a}", a = tcx.mk_re_var(a))
+                }
+            };
+            guar = Some(
+                struct_span_err!(
+                    tcx.sess,
+                    error.origin().span(),
+                    E0367,
+                    "`Drop` impl requires `{outlives}` \
+                    but the {self_descr} it is implemented for does not",
+                )
+                .span_note(item_span, "the implementor must specify the same requirement")
+                .emit(),
+            );
         }
+        return Err(guar.unwrap());
     }
 
-    fn consts(
-        &mut self,
-        a: ty::Const<'tcx>,
-        b: ty::Const<'tcx>,
-    ) -> RelateResult<'tcx, ty::Const<'tcx>> {
-        debug!("SimpleEqRelation::consts(a={:?}, b={:?})", a, b);
-        ty::relate::super_relate_consts(self, a, b)
-    }
-
-    fn binders<T>(
-        &mut self,
-        a: ty::Binder<'tcx, T>,
-        b: ty::Binder<'tcx, T>,
-    ) -> RelateResult<'tcx, ty::Binder<'tcx, T>>
-    where
-        T: Relate<'tcx>,
-    {
-        debug!("SimpleEqRelation::binders({:?}: {:?}", a, b);
-
-        // Anonymizing the LBRs is necessary to solve (Issue #59497).
-        // After we do so, it should be totally fine to skip the binders.
-        let anon_a = self.tcx.anonymize_bound_vars(a);
-        let anon_b = self.tcx.anonymize_bound_vars(b);
-        self.relate(anon_a.skip_binder(), anon_b.skip_binder())?;
-
-        Ok(a)
-    }
+    Ok(())
 }

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

@@ -102,6 +102,17 @@ pub enum RegionResolutionError<'tcx> {
     ),
 }
 
+impl<'tcx> RegionResolutionError<'tcx> {
+    pub fn origin(&self) -> &SubregionOrigin<'tcx> {
+        match self {
+            RegionResolutionError::ConcreteFailure(origin, _, _)
+            | RegionResolutionError::GenericBoundFailure(origin, _, _)
+            | RegionResolutionError::SubSupConflict(_, _, origin, _, _, _, _)
+            | RegionResolutionError::UpperBoundUniverseConflict(_, _, _, origin, _) => origin,
+        }
+    }
+}
+
 struct RegionAndOrigin<'tcx> {
     region: Region<'tcx>,
     origin: SubregionOrigin<'tcx>,