Auto merge of rust-lang#136539 - matthewjasper:late-normalize-errors, r=compiler-errors

Emit dropck normalization errors in borrowck

Borrowck generally assumes that any queries it runs for type checking will succeed, thinking that HIR typeck will have errored first if there was a problem. However as of rust-lang#98641, dropck isn't run on HIR, so there's no direct guarantee that it doesn't error. While a type being well-formed might be expected to ensure that its fields are well-formed, this is not the case for types containing a type projection:

```rust
pub trait AuthUser {
    type Id;
}

pub trait AuthnBackend {
    type User: AuthUser;
}

pub struct AuthSession<Backend: AuthnBackend> {
    data: Option<<<Backend as AuthnBackend>::User as AuthUser>::Id>,
}

pub trait Authz: Sized {
    type AuthnBackend: AuthnBackend<User = Self>;
}

pub fn run_query<User: Authz>(auth: AuthSession<User::AuthnBackend>) {}
// ^ No User: AuthUser bound is required or inferred.
```

While improvements to trait solving might fix this in the future, for now we go for a pragmatic solution of emitting an error from borrowck (by rerunning dropck outside of a query) and making drop elaboration check if an error has been emitted previously before panicking for a failed normalization.

Closes rust-lang#103899
Closes rust-lang#135039

r? `@compiler-errors` (feel free to re-assign)

Author: bors

compiler/rustc_borrowck/src/type_check/liveness/trace.rs

@@ -3,15 +3,19 @@ use rustc_index::bit_set::DenseBitSet;
 use rustc_index::interval::IntervalSet;
 use rustc_infer::infer::canonical::QueryRegionConstraints;
 use rustc_infer::infer::outlives::for_liveness;
-use rustc_middle::mir::{BasicBlock, Body, ConstraintCategory, Local, Location};
+use rustc_middle::mir::{BasicBlock, Body, ConstraintCategory, HasLocalDecls, Local, Location};
+use rustc_middle::span_bug;
 use rustc_middle::traits::query::DropckOutlivesResult;
 use rustc_middle::ty::relate::Relate;
 use rustc_middle::ty::{Ty, TyCtxt, TypeVisitable, TypeVisitableExt};
 use rustc_mir_dataflow::ResultsCursor;
 use rustc_mir_dataflow::impls::MaybeInitializedPlaces;
 use rustc_mir_dataflow::move_paths::{HasMoveData, MoveData, MovePathIndex};
 use rustc_mir_dataflow::points::{DenseLocationMap, PointIndex};
-use rustc_span::DUMMY_SP;
+use rustc_span::{DUMMY_SP, Span};
+use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
+use rustc_trait_selection::traits::ObligationCtxt;
+use rustc_trait_selection::traits::query::dropck_outlives;
 use rustc_trait_selection::traits::query::type_op::{DropckOutlives, TypeOp, TypeOpOutput};
 use tracing::debug;
 
@@ -162,9 +166,10 @@ impl<'a, 'typeck, 'b, 'tcx> LivenessResults<'a, 'typeck, 'b, 'tcx> {
     fn dropck_boring_locals(&mut self, boring_locals: Vec<Local>) {
         for local in boring_locals {
             let local_ty = self.cx.body.local_decls[local].ty;
+            let local_span = self.cx.body.local_decls[local].source_info.span;
             let drop_data = self.cx.drop_data.entry(local_ty).or_insert_with({
                 let typeck = &self.cx.typeck;
-                move || LivenessContext::compute_drop_data(typeck, local_ty)
+                move || LivenessContext::compute_drop_data(typeck, local_ty, local_span)
             });
 
             drop_data.dropck_result.report_overflows(
@@ -522,9 +527,10 @@ impl<'tcx> LivenessContext<'_, '_, '_, 'tcx> {
             values::pretty_print_points(self.location_map, live_at.iter()),
         );
 
+        let local_span = self.body.local_decls()[dropped_local].source_info.span;
         let drop_data = self.drop_data.entry(dropped_ty).or_insert_with({
             let typeck = &self.typeck;
-            move || Self::compute_drop_data(typeck, dropped_ty)
+            move || Self::compute_drop_data(typeck, dropped_ty, local_span)
         });
 
         if let Some(data) = &drop_data.region_constraint_data {
@@ -589,19 +595,44 @@ impl<'tcx> LivenessContext<'_, '_, '_, 'tcx> {
         }
     }
 
-    fn compute_drop_data(typeck: &TypeChecker<'_, 'tcx>, dropped_ty: Ty<'tcx>) -> DropData<'tcx> {
-        debug!("compute_drop_data(dropped_ty={:?})", dropped_ty,);
+    fn compute_drop_data(
+        typeck: &TypeChecker<'_, 'tcx>,
+        dropped_ty: Ty<'tcx>,
+        span: Span,
+    ) -> DropData<'tcx> {
+        debug!("compute_drop_data(dropped_ty={:?})", dropped_ty);
+
+        let op = typeck.infcx.param_env.and(DropckOutlives { dropped_ty });
 
-        match typeck
-            .infcx
-            .param_env
-            .and(DropckOutlives { dropped_ty })
-            .fully_perform(typeck.infcx, DUMMY_SP)
-        {
+        match op.fully_perform(typeck.infcx, DUMMY_SP) {
             Ok(TypeOpOutput { output, constraints, .. }) => {
                 DropData { dropck_result: output, region_constraint_data: constraints }
             }
-            Err(_) => DropData { dropck_result: Default::default(), region_constraint_data: None },
+            Err(_) => {
+                // We don't run dropck on HIR, and dropck looks inside fields of
+                // types, so there's no guarantee that it succeeds. We also
+                // can't rely on the the `ErrorGuaranteed` from `fully_perform` here
+                // because it comes from delay_span_bug.
+                let ocx = ObligationCtxt::new_with_diagnostics(&typeck.infcx);
+                let errors =
+                    match dropck_outlives::compute_dropck_outlives_with_errors(&ocx, op, span) {
+                        Ok(_) => ocx.select_all_or_error(),
+                        Err(e) => {
+                            if e.is_empty() {
+                                ocx.select_all_or_error()
+                            } else {
+                                e
+                            }
+                        }
+                    };
+
+                if !errors.is_empty() {
+                    typeck.infcx.err_ctxt().report_fulfillment_errors(errors);
+                } else {
+                    span_bug!(span, "Rerunning drop data query produced no error.");
+                }
+                DropData { dropck_result: Default::default(), region_constraint_data: None }
+            }
         }
     }
 }

compiler/rustc_middle/src/query/mod.rs

@@ -802,7 +802,7 @@ rustc_queries! {
 
     query adt_dtorck_constraint(
         key: DefId
-    ) -> Result<&'tcx DropckConstraint<'tcx>, NoSolution> {
+    ) -> &'tcx DropckConstraint<'tcx> {
         desc { |tcx| "computing drop-check constraints for `{}`", tcx.def_path_str(key) }
     }
 

compiler/rustc_mir_transform/src/elaborate_drop.rs

@@ -266,8 +266,21 @@ where
                 let tcx = self.tcx();
 
                 assert_eq!(self.elaborator.typing_env().typing_mode, ty::TypingMode::PostAnalysis);
-                let field_ty =
-                    tcx.normalize_erasing_regions(self.elaborator.typing_env(), f.ty(tcx, args));
+                // The type error for normalization may have been in dropck: see
+                // `compute_drop_data` in rustc_borrowck, in which case we wouldn't have
+                // deleted the MIR body and could have an error here as well.
+                let field_ty = match tcx
+                    .try_normalize_erasing_regions(self.elaborator.typing_env(), f.ty(tcx, args))
+                {
+                    Ok(t) => t,
+                    Err(_) => Ty::new_error(
+                        self.tcx(),
+                        self.elaborator
+                            .body()
+                            .tainted_by_errors
+                            .expect("Error in drop elaboration not found by dropck."),
+                    ),
+                };
 
                 (tcx.mk_place_field(base_place, field, field_ty), subpath)
             })

compiler/rustc_trait_selection/src/traits/query/dropck_outlives.rs

@@ -2,12 +2,13 @@ use rustc_data_structures::fx::FxHashSet;
 use rustc_infer::traits::query::type_op::DropckOutlives;
 use rustc_middle::traits::query::{DropckConstraint, DropckOutlivesResult};
 use rustc_middle::ty::{self, EarlyBinder, ParamEnvAnd, Ty, TyCtxt};
-use rustc_span::{DUMMY_SP, Span};
+use rustc_span::Span;
 use tracing::{debug, instrument};
 
+use crate::solve::NextSolverError;
 use crate::traits::query::NoSolution;
 use crate::traits::query::normalize::QueryNormalizeExt;
-use crate::traits::{Normalized, ObligationCause, ObligationCtxt};
+use crate::traits::{FromSolverError, Normalized, ObligationCause, ObligationCtxt};
 
 /// This returns true if the type `ty` is "trivial" for
 /// dropck-outlives -- that is, if it doesn't require any types to
@@ -93,6 +94,20 @@ pub fn compute_dropck_outlives_inner<'tcx>(
     goal: ParamEnvAnd<'tcx, DropckOutlives<'tcx>>,
     span: Span,
 ) -> Result<DropckOutlivesResult<'tcx>, NoSolution> {
+    match compute_dropck_outlives_with_errors(ocx, goal, span) {
+        Ok(r) => Ok(r),
+        Err(_) => Err(NoSolution),
+    }
+}
+
+pub fn compute_dropck_outlives_with_errors<'tcx, E>(
+    ocx: &ObligationCtxt<'_, 'tcx, E>,
+    goal: ParamEnvAnd<'tcx, DropckOutlives<'tcx>>,
+    span: Span,
+) -> Result<DropckOutlivesResult<'tcx>, Vec<E>>
+where
+    E: FromSolverError<'tcx, NextSolverError<'tcx>>,
+{
     let tcx = ocx.infcx.tcx;
     let ParamEnvAnd { param_env, value: DropckOutlives { dropped_ty } } = goal;
 
@@ -149,11 +164,11 @@ pub fn compute_dropck_outlives_inner<'tcx>(
         dtorck_constraint_for_ty_inner(
             tcx,
             ocx.infcx.typing_env(param_env),
-            DUMMY_SP,
+            span,
             depth,
             ty,
             &mut constraints,
-        )?;
+        );
 
         // "outlives" represent types/regions that may be touched
         // by a destructor.
@@ -173,11 +188,20 @@ pub fn compute_dropck_outlives_inner<'tcx>(
         // do not themselves define a destructor", more or less. We have
         // to push them onto the stack to be expanded.
         for ty in constraints.dtorck_types.drain(..) {
-            let Normalized { value: ty, obligations } =
-                ocx.infcx.at(&cause, param_env).query_normalize(ty)?;
-            ocx.register_obligations(obligations);
+            let ty = if let Ok(Normalized { value: ty, obligations }) =
+                ocx.infcx.at(&cause, param_env).query_normalize(ty)
+            {
+                ocx.register_obligations(obligations);
+
+                debug!("dropck_outlives: ty from dtorck_types = {:?}", ty);
+                ty
+            } else {
+                ocx.deeply_normalize(&cause, param_env, ty)?;
 
-            debug!("dropck_outlives: ty from dtorck_types = {:?}", ty);
+                let errors = ocx.select_where_possible();
+                debug!("normalize errors: {ty} ~> {errors:#?}");
+                return Err(errors);
+            };
 
             match ty.kind() {
                 // All parameters live for the duration of the
@@ -213,14 +237,14 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>(
     depth: usize,
     ty: Ty<'tcx>,
     constraints: &mut DropckConstraint<'tcx>,
-) -> Result<(), NoSolution> {
+) {
     if !tcx.recursion_limit().value_within_limit(depth) {
         constraints.overflows.push(ty);
-        return Ok(());
+        return;
     }
 
     if trivial_dropck_outlives(tcx, ty) {
-        return Ok(());
+        return;
     }
 
     match ty.kind() {
@@ -244,22 +268,20 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>(
             // single-element containers, behave like their element
             rustc_data_structures::stack::ensure_sufficient_stack(|| {
                 dtorck_constraint_for_ty_inner(tcx, typing_env, span, depth + 1, *ety, constraints)
-            })?;
+            });
         }
 
         ty::Tuple(tys) => rustc_data_structures::stack::ensure_sufficient_stack(|| {
             for ty in tys.iter() {
-                dtorck_constraint_for_ty_inner(tcx, typing_env, span, depth + 1, ty, constraints)?;
+                dtorck_constraint_for_ty_inner(tcx, typing_env, span, depth + 1, ty, constraints);
             }
-            Ok::<_, NoSolution>(())
-        })?,
+        }),
 
         ty::Closure(_, args) => rustc_data_structures::stack::ensure_sufficient_stack(|| {
             for ty in args.as_closure().upvar_tys() {
-                dtorck_constraint_for_ty_inner(tcx, typing_env, span, depth + 1, ty, constraints)?;
+                dtorck_constraint_for_ty_inner(tcx, typing_env, span, depth + 1, ty, constraints);
             }
-            Ok::<_, NoSolution>(())
-        })?,
+        }),
 
         ty::CoroutineClosure(_, args) => {
             rustc_data_structures::stack::ensure_sufficient_stack(|| {
@@ -271,10 +293,9 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>(
                         depth + 1,
                         ty,
                         constraints,
-                    )?;
+                    );
                 }
-                Ok::<_, NoSolution>(())
-            })?
+            })
         }
 
         ty::Coroutine(_, args) => {
@@ -313,7 +334,7 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>(
 
         ty::Adt(def, args) => {
             let DropckConstraint { dtorck_types, outlives, overflows } =
-                tcx.at(span).adt_dtorck_constraint(def.did())?;
+                tcx.at(span).adt_dtorck_constraint(def.did());
             // FIXME: we can try to recursively `dtorck_constraint_on_ty`
             // there, but that needs some way to handle cycles.
             constraints
@@ -346,9 +367,7 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>(
         ty::Placeholder(..) | ty::Bound(..) | ty::Infer(..) | ty::Error(_) => {
             // By the time this code runs, all type variables ought to
             // be fully resolved.
-            return Err(NoSolution);
+            tcx.dcx().span_delayed_bug(span, format!("Unresolved type in dropck: {:?}.", ty));
         }
     }
-
-    Ok(())
 }

compiler/rustc_traits/src/dropck_outlives.rs

@@ -30,10 +30,7 @@ fn dropck_outlives<'tcx>(
 }
 
 /// Calculates the dtorck constraint for a type.
-pub(crate) fn adt_dtorck_constraint(
-    tcx: TyCtxt<'_>,
-    def_id: DefId,
-) -> Result<&DropckConstraint<'_>, NoSolution> {
+pub(crate) fn adt_dtorck_constraint(tcx: TyCtxt<'_>, def_id: DefId) -> &DropckConstraint<'_> {
     let def = tcx.adt_def(def_id);
     let span = tcx.def_span(def_id);
     let typing_env = ty::TypingEnv::non_body_analysis(tcx, def_id);
@@ -52,20 +49,20 @@ pub(crate) fn adt_dtorck_constraint(
             overflows: vec![],
         };
         debug!("dtorck_constraint: {:?} => {:?}", def, result);
-        return Ok(tcx.arena.alloc(result));
+        return tcx.arena.alloc(result);
     }
 
     let mut result = DropckConstraint::empty();
     for field in def.all_fields() {
         let fty = tcx.type_of(field.did).instantiate_identity();
-        dtorck_constraint_for_ty_inner(tcx, typing_env, span, 0, fty, &mut result)?;
+        dtorck_constraint_for_ty_inner(tcx, typing_env, span, 0, fty, &mut result);
     }
     result.outlives.extend(tcx.destructor_constraints(def));
     dedup_dtorck_constraint(&mut result);
 
     debug!("dtorck_constraint: {:?} => {:?}", def, result);
 
-    Ok(tcx.arena.alloc(result))
+    tcx.arena.alloc(result)
 }
 
 fn dedup_dtorck_constraint(c: &mut DropckConstraint<'_>) {

tests/crashes/103899.rs

@@ -0,0 +1,23 @@
+// Test that we don't ICE for a typeck error that only shows up in dropck
+// Version where the normalization error is an ambiguous trait implementation.
+// <[T] as ToOwned>::Owned is ambiguous on whether to use T: Clone or [T]::Clone.
+// Regression test for #105299
+
+pub trait Foo: Clone {}
+
+pub struct Bar<'a, T: Clone> {
+    pub cow: std::borrow::Cow<'a, [T]>,
+
+    pub THIS_CAUSES_ICE: (),
+}
+
+impl<T> Bar<'_, T>
+where
+    T: Clone,
+    [T]: Foo,
+{
+    pub fn MOVES_SELF(self) {}
+    //~^ ERROR type annotations needed
+}
+
+pub fn main() {}