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

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,18 @@ 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::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 +165,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 +526,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 +594,45 @@ 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, true,
+                ) {
+                    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 {
+                    rustc_middle::span_bug!(span, "Rerunning drop data query produced no error.");
+                }
+                DropData { dropck_result: Default::default(), region_constraint_data: None }
+            }
         }
     }
 }

compiler/rustc_mir_dataflow/src/elaborate_drops.rs

@@ -274,8 +274,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,21 @@ 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, false) {
+        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,
+    report_errors: bool,
+) -> 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;
 
@@ -146,14 +162,17 @@ pub fn compute_dropck_outlives_inner<'tcx>(
             result.overflows.len(),
             ty_stack.len()
         );
-        dtorck_constraint_for_ty_inner(
+        match dtorck_constraint_for_ty_inner(
             tcx,
             ocx.infcx.typing_env(param_env),
-            DUMMY_SP,
+            span,
             depth,
             ty,
             &mut constraints,
-        )?;
+        ) {
+            Err(_) => return Err(Vec::new()),
+            _ => (),
+        };
 
         // "outlives" represent types/regions that may be touched
         // by a destructor.
@@ -173,11 +192,25 @@ 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 report_errors {
+                let normalized_ty = ocx.deeply_normalize(&cause, param_env, ty)?;
 
-            debug!("dropck_outlives: ty from dtorck_types = {:?}", ty);
+                let errors = ocx.select_where_possible();
+                if !errors.is_empty() {
+                    debug!("failed to normalize dtorck type: {ty} ~> {errors:#?}");
+                    return Err(errors);
+                }
+                normalized_ty
+            } else 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 {
+                return Err(Vec::new());
+            };
 
             match ty.kind() {
                 // All parameters live for the duration of the

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() {}

tests/crashes/105299.rs

@@ -0,0 +1,11 @@
+error[E0284]: type annotations needed
+  --> $DIR/dropck-only-error-ambiguity.rs:19:23
+   |
+LL |     pub fn MOVES_SELF(self) {}
+   |                       ^^^^ cannot infer type
+   |
+   = note: cannot satisfy `<[T] as ToOwned>::Owned == _`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0284`.

tests/ui/dropck/dropck-only-error-ambiguity.rs

@@ -1,7 +1,6 @@
-//@ known-bug: #135039
-//@ edition:2021
-
-pub type UserId<Backend> = <<Backend as AuthnBackend>::User as AuthUser>::Id;
+// Test that we don't ICE for a typeck error that only shows up in dropck
+// issue #135039
+//@ edition:2018
 
 pub trait AuthUser {
     type Id;
@@ -13,7 +12,7 @@ pub trait AuthnBackend {
 
 pub struct AuthSession<Backend: AuthnBackend> {
     user: Option<Backend::User>,
-    data: Option<UserId<Backend>>,
+    data: Option<<<Backend as AuthnBackend>::User as AuthUser>::Id>,
 }
 
 pub trait Authz: Sized {
@@ -27,8 +26,12 @@ pub trait Query<User: Authz> {
 
 pub async fn run_query<User: Authz, Q: Query<User> + 'static>(
     auth: AuthSession<User::AuthnBackend>,
+    //~^ ERROR the trait bound `User: AuthUser` is not satisfied [E0277]
+    //~| ERROR the trait bound `User: AuthUser` is not satisfied [E0277]
     query: Q,
 ) -> Result<Q::Output, ()> {
     let user = auth.user;
     query.run().await
 }
+
+fn main() {}

tests/ui/dropck/dropck-only-error-ambiguity.stderr

@@ -0,0 +1,15 @@
+error[E0277]: the trait bound `User: AuthUser` is not satisfied
+  --> $DIR/dropck-only-error-async.rs:28:5
+   |
+LL |     auth: AuthSession<User::AuthnBackend>,
+   |     ^^^^ the trait `AuthUser` is not implemented for `User`
+
+error[E0277]: the trait bound `User: AuthUser` is not satisfied
+  --> $DIR/dropck-only-error-async.rs:28:5
+   |
+LL |     auth: AuthSession<User::AuthnBackend>,
+   |     ^^^^ the trait `AuthUser` is not implemented for `User`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0277`.

tests/crashes/135039.rs renamed to tests/ui/dropck/dropck-only-error-async.rs

@@ -1,6 +1,6 @@
-//@ known-bug: #91985
-
-#![feature(generic_associated_types)]
+// Test that we don't ICE for a typeck error that only shows up in dropck
+// Version that uses a generic associated type
+// Regression test for #91985
 
 pub trait Trait1 {
     type Associated: Ord;
@@ -22,7 +22,7 @@ impl GatTrait for GatStruct {
 
 pub struct OuterStruct<T1: Trait1, T2: Trait2> {
     inner: InnerStruct<T2, GatStruct>,
-    t1:    T1,
+    t1: T1,
 }
 
 pub struct InnerStruct<T: Trait2, G: GatTrait> {
@@ -35,6 +35,7 @@ where
     T2: Trait2<Associated = T1::Associated>,
 {
     pub fn new() -> Self {
+        //~^ ERROR the trait bound `<T1 as Trait1>::Associated: Clone` is not satisfied
         todo!()
     }
 }