Skip to content

Commit

Permalink
Auto merge of rust-lang#136539 - matthewjasper:late-normalize-errors,…
Browse files Browse the repository at this point in the history
… 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)
  • Loading branch information
bors committed Feb 19, 2025
2 parents 5986ff0 + 49cf00c commit ed49386
Show file tree
Hide file tree
Showing 20 changed files with 262 additions and 109 deletions.
57 changes: 44 additions & 13 deletions compiler/rustc_borrowck/src/type_check/liveness/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 }
}
}
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
}

Expand Down
17 changes: 15 additions & 2 deletions compiler/rustc_mir_transform/src/elaborate_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
69 changes: 44 additions & 25 deletions compiler/rustc_trait_selection/src/traits/query/dropck_outlives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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() {
Expand All @@ -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(|| {
Expand All @@ -271,10 +293,9 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>(
depth + 1,
ty,
constraints,
)?;
);
}
Ok::<_, NoSolution>(())
})?
})
}

ty::Coroutine(_, args) => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(())
}
11 changes: 4 additions & 7 deletions compiler/rustc_traits/src/dropck_outlives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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<'_>) {
Expand Down
27 changes: 0 additions & 27 deletions tests/crashes/103899.rs

This file was deleted.

19 changes: 0 additions & 19 deletions tests/crashes/105299.rs

This file was deleted.

23 changes: 23 additions & 0 deletions tests/ui/dropck/dropck-only-error-ambiguity.rs
Original file line number Diff line number Diff line change
@@ -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() {}
Loading

0 comments on commit ed49386

Please sign in to comment.