Skip to content

refactor writeback: emit normalization errors with new solver #118751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ fn check_item_type(tcx: TyCtxt<'_>, id: hir::ItemId) {
match assoc_item.kind {
ty::AssocKind::Fn => {
let abi = tcx.fn_sig(assoc_item.def_id).skip_binder().abi();
fn_maybe_err(tcx, assoc_item.ident(tcx).span, abi);
forbid_intrinsic_abi(tcx, assoc_item.ident(tcx).span, abi);
}
ty::AssocKind::Type if assoc_item.defaultness(tcx).has_value() => {
let trait_args = GenericArgs::identity_for_item(tcx, id.owner_id);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ fn get_owner_return_paths(
/// Forbid defining intrinsics in Rust code,
/// as they must always be defined by the compiler.
// FIXME: Move this to a more appropriate place.
pub fn fn_maybe_err(tcx: TyCtxt<'_>, sp: Span, abi: Abi) {
pub fn forbid_intrinsic_abi(tcx: TyCtxt<'_>, sp: Span, abi: Abi) {
if let Abi::RustIntrinsic | Abi::PlatformIntrinsic = abi {
tcx.sess.span_err(sp, "intrinsic must be in `extern \"rust-intrinsic\" { ... }` block");
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::intravisit::Visitor;
use rustc_hir::lang_items::LangItem;
use rustc_hir_analysis::check::{check_function_signature, fn_maybe_err};
use rustc_hir_analysis::check::{check_function_signature, forbid_intrinsic_abi};
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::RegionVariableOrigin;
use rustc_middle::ty::{self, Binder, Ty, TyCtxt};
Expand Down Expand Up @@ -53,7 +53,7 @@ pub(super) fn check_fn<'a, 'tcx>(

let span = body.value.span;

fn_maybe_err(tcx, span, fn_sig.abi);
forbid_intrinsic_abi(tcx, span, fn_sig.abi);

if let Some(kind) = body.coroutine_kind
&& can_be_coroutine.is_some()
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_hir_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,6 @@ fn typeck_with_fallback<'tcx>(

fcx.check_asms();

fcx.infcx.skip_region_resolution();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... seems a bit sketch. Idk if I have any better ideas other than this though...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine.

We only did this to enable the use of fully_resolve in writeback to also resolve regions. But given that we resolve all regions to 'erased anyways, this is pretty pointless and resolve_vars_if_possible is good enough.


let typeck_results = fcx.resolve_type_vars_in_body(body);

// Consistency check our TypeckResults instance can hold all ItemLocalIds
Expand Down
129 changes: 68 additions & 61 deletions compiler/rustc_hir_typeck/src/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,16 @@ use rustc_errors::{ErrorGuaranteed, StashKey};
use rustc_hir as hir;
use rustc_hir::intravisit::{self, Visitor};
use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282;
use rustc_middle::traits::ObligationCause;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, PointerCoercion};
use rustc_middle::ty::fold::{TypeFoldable, TypeFolder};
use rustc_middle::ty::visit::TypeVisitableExt;
use rustc_middle::ty::TypeSuperFoldable;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_trait_selection::solve;
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt;

use std::mem;

Expand Down Expand Up @@ -695,24 +699,22 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
}
}

fn resolve<T>(&mut self, x: T, span: &dyn Locatable) -> T
fn resolve<T>(&mut self, value: T, span: &dyn Locatable) -> T
where
T: TypeFoldable<TyCtxt<'tcx>>,
{
let mut resolver = Resolver::new(self.fcx, span, self.body);
let x = x.fold_with(&mut resolver);
if cfg!(debug_assertions) && x.has_infer() {
span_bug!(span.to_span(self.fcx.tcx), "writeback: `{:?}` has inference variables", x);
}
let value = self.fcx.resolve_vars_if_possible(value);
let value = value.fold_with(&mut Resolver::new(self.fcx, span, self.body));
assert!(!value.has_infer());

// We may have introduced e.g. `ty::Error`, if inference failed, make sure
// to mark the `TypeckResults` as tainted in that case, so that downstream
// users of the typeck results don't produce extra errors, or worse, ICEs.
if let Some(e) = resolver.replaced_with_error {
self.typeck_results.tainted_by_errors = Some(e);
if let Err(guar) = value.error_reported() {
self.typeck_results.tainted_by_errors = Some(guar);
}

x
value
}
}

Expand All @@ -732,15 +734,13 @@ impl Locatable for hir::HirId {
}
}

/// The Resolver. This is the type folding engine that detects
/// unresolved types and so forth.
struct Resolver<'cx, 'tcx> {
fcx: &'cx FnCtxt<'cx, 'tcx>,
span: &'cx dyn Locatable,
body: &'tcx hir::Body<'tcx>,

/// Set to `Some` if any `Ty` or `ty::Const` had to be replaced with an `Error`.
replaced_with_error: Option<ErrorGuaranteed>,
/// Whether we should normalize using the new solver, disabled
/// both when using the old solver and when resolving predicates.
should_normalize: bool,
}

impl<'cx, 'tcx> Resolver<'cx, 'tcx> {
Expand All @@ -749,7 +749,7 @@ impl<'cx, 'tcx> Resolver<'cx, 'tcx> {
span: &'cx dyn Locatable,
body: &'tcx hir::Body<'tcx>,
) -> Resolver<'cx, 'tcx> {
Resolver { fcx, span, body, replaced_with_error: None }
Resolver { fcx, span, body, should_normalize: fcx.next_trait_solver() }
}

fn report_error(&self, p: impl Into<ty::GenericArg<'tcx>>) -> ErrorGuaranteed {
Expand All @@ -768,64 +768,71 @@ impl<'cx, 'tcx> Resolver<'cx, 'tcx> {
.emit(),
}
}

fn handle_term<T>(
&mut self,
value: T,
outer_exclusive_binder: impl FnOnce(T) -> ty::DebruijnIndex,
new_err: impl Fn(TyCtxt<'tcx>, ErrorGuaranteed) -> T,
) -> T
where
T: Into<ty::GenericArg<'tcx>> + TypeSuperFoldable<TyCtxt<'tcx>> + Copy,
{
let tcx = self.fcx.tcx;
// We must deeply normalize in the new solver, since later lints
// expect that types that show up in the typeck are fully
// normalized.
let value = if self.should_normalize {
let body_id = tcx.hir().body_owner_def_id(self.body.id());
let cause = ObligationCause::misc(self.span.to_span(tcx), body_id);
let at = self.fcx.at(&cause, self.fcx.param_env);
let universes = vec![None; outer_exclusive_binder(value).as_usize()];
solve::deeply_normalize_with_skipped_universes(at, value, universes).unwrap_or_else(
|errors| {
let guar = self.fcx.err_ctxt().report_fulfillment_errors(errors);
new_err(tcx, guar)
},
)
} else {
value
};

if value.has_non_region_infer() {
let guar = self.report_error(value);
new_err(tcx, guar)
} else {
tcx.fold_regions(value, |_, _| tcx.lifetimes.re_erased)
}
}
}

impl<'cx, 'tcx> TypeFolder<TyCtxt<'tcx>> for Resolver<'cx, 'tcx> {
fn interner(&self) -> TyCtxt<'tcx> {
self.fcx.tcx
}

fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
let tcx = self.fcx.tcx;
match self.fcx.fully_resolve(t) {
Ok(t) if self.fcx.next_trait_solver() => {
// We must normalize erasing regions here, since later lints
// expect that types that show up in the typeck are fully
// normalized.
if let Ok(t) = tcx.try_normalize_erasing_regions(self.fcx.param_env, t) {
t
} else {
tcx.fold_regions(t, |_, _| tcx.lifetimes.re_erased)
}
}
Ok(t) => {
// Do not anonymize late-bound regions
// (e.g. keep `for<'a>` named `for<'a>`).
// This allows NLL to generate error messages that
// refer to the higher-ranked lifetime names written by the user.
tcx.fold_regions(t, |_, _| tcx.lifetimes.re_erased)
}
Err(_) => {
debug!("Resolver::fold_ty: input type `{:?}` not fully resolvable", t);
let e = self.report_error(t);
self.replaced_with_error = Some(e);
Ty::new_error(self.fcx.tcx, e)
}
}
}

fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
debug_assert!(!r.is_bound(), "Should not be resolving bound region.");
self.fcx.tcx.lifetimes.re_erased
}

fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
self.handle_term(ty, Ty::outer_exclusive_binder, Ty::new_error)
}

fn fold_const(&mut self, ct: ty::Const<'tcx>) -> ty::Const<'tcx> {
match self.fcx.fully_resolve(ct) {
Ok(ct) => self.fcx.tcx.erase_regions(ct),
Err(_) => {
debug!("Resolver::fold_const: input const `{:?}` not fully resolvable", ct);
let e = self.report_error(ct);
self.replaced_with_error = Some(e);
ty::Const::new_error(self.fcx.tcx, e, ct.ty())
}
}
self.handle_term(ct, ty::Const::outer_exclusive_binder, |tcx, guar| {
ty::Const::new_error(tcx, guar, ct.ty())
})
}
}

///////////////////////////////////////////////////////////////////////////
// During type check, we store promises with the result of trait
// lookup rather than the actual results (because the results are not
// necessarily available immediately). These routines unwind the
// promises. It is expected that we will have already reported any
// errors that may be encountered, so if the promises store an error,
// a dummy result is returned.
fn fold_predicate(&mut self, predicate: ty::Predicate<'tcx>) -> ty::Predicate<'tcx> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When do we resolve predicates during writeback?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right

// Do not normalize predicates in the new solver. The new solver is
// supposed to handle unnormalized predicates and incorrectly normalizing
// them can be unsound, e.g. for `WellFormed` predicates.
let prev = mem::replace(&mut self.should_normalize, false);
let predicate = predicate.super_fold_with(self);
self.should_normalize = prev;
predicate
}
}
28 changes: 1 addition & 27 deletions compiler/rustc_infer/src/infer/outlives/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use self::env::OutlivesEnvironment;
use super::region_constraints::RegionConstraintData;
use super::{InferCtxt, RegionResolutionError};
use crate::infer::free_regions::RegionRelations;
use crate::infer::lexical_region_resolve::{self, LexicalRegionResolutions};
use crate::infer::lexical_region_resolve;
use rustc_middle::traits::query::OutlivesBound;
use rustc_middle::ty;

Expand Down Expand Up @@ -37,32 +37,6 @@ pub fn explicit_outlives_bounds<'tcx>(
}

impl<'tcx> InferCtxt<'tcx> {
pub fn skip_region_resolution(&self) {
let (var_infos, _) = {
let mut inner = self.inner.borrow_mut();
let inner = &mut *inner;
// Note: `inner.region_obligations` may not be empty, because we
// didn't necessarily call `process_registered_region_obligations`.
// This is okay, because that doesn't introduce new vars.
inner
.region_constraint_storage
.take()
.expect("regions already resolved")
.with_log(&mut inner.undo_log)
.into_infos_and_data()
};

let lexical_region_resolutions = LexicalRegionResolutions {
values: rustc_index::IndexVec::from_elem_n(
crate::infer::lexical_region_resolve::VarValue::Value(self.tcx.lifetimes.re_erased),
var_infos.len(),
),
};

let old_value = self.lexical_region_resolutions.replace(Some(lexical_region_resolutions));
assert!(old_value.is_none());
}

/// Process the region constraints and return any errors that
/// result. After this, no more unification operations should be
/// done -- or the compiler will panic -- but it is legal to use
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_trait_selection/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,8 @@ mod trait_goals;

pub use eval_ctxt::{EvalCtxt, GenerateProofTree, InferCtxtEvalExt, InferCtxtSelectExt};
pub use fulfill::FulfillmentCtxt;
pub(crate) use normalize::{
deeply_normalize, deeply_normalize_for_diagnostics, deeply_normalize_with_skipped_universes,
};
pub(crate) use normalize::deeply_normalize_for_diagnostics;
pub use normalize::{deeply_normalize, deeply_normalize_with_skipped_universes};

#[derive(Debug, Clone, Copy)]
enum SolverMode {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/solve/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use super::FulfillmentCtxt;

/// Deeply normalize all aliases in `value`. This does not handle inference and expects
/// its input to be already fully resolved.
pub(crate) fn deeply_normalize<'tcx, T: TypeFoldable<TyCtxt<'tcx>>>(
pub fn deeply_normalize<'tcx, T: TypeFoldable<TyCtxt<'tcx>>>(
at: At<'_, 'tcx>,
value: T,
) -> Result<T, Vec<FulfillmentError<'tcx>>> {
Expand All @@ -31,7 +31,7 @@ pub(crate) fn deeply_normalize<'tcx, T: TypeFoldable<TyCtxt<'tcx>>>(
/// Additionally takes a list of universes which represents the binders which have been
/// entered before passing `value` to the function. This is currently needed for
/// `normalize_erasing_regions`, which skips binders as it walks through a type.
pub(crate) fn deeply_normalize_with_skipped_universes<'tcx, T: TypeFoldable<TyCtxt<'tcx>>>(
pub fn deeply_normalize_with_skipped_universes<'tcx, T: TypeFoldable<TyCtxt<'tcx>>>(
at: At<'_, 'tcx>,
value: T,
universes: Vec<Option<UniverseIndex>>,
Expand Down
1 change: 1 addition & 0 deletions tests/ui/traits/new-solver/alias-bound-unsound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@ fn main() {
//~| ERROR overflow evaluating the requirement `<() as Foo>::Item well-formed`
//~| ERROR overflow evaluating the requirement `String <: <() as Foo>::Item`
//~| ERROR overflow evaluating the requirement `&<() as Foo>::Item well-formed`
//~| ERROR overflow evaluating the requirement `<() as Foo>::Item normalizes-to _`
println!("{x}");
}
11 changes: 10 additions & 1 deletion tests/ui/traits/new-solver/alias-bound-unsound.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ LL | drop(<() as Foo>::copy_me(&x));
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`alias_bound_unsound`)

error: aborting due to 6 previous errors
error[E0275]: overflow evaluating the requirement `<() as Foo>::Item normalizes-to _`
--> $DIR/alias-bound-unsound.rs:24:10
|
LL | drop(<() as Foo>::copy_me(&x));
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`alias_bound_unsound`)
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 7 previous errors

For more information about this error, try `rustc --explain E0275`.
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// compile-flags: -Ztrait-solver=next
// known-bug: trait-system-refactor-initiative#60
// dont-check-failure-status
// dont-check-compiler-stderr

// Generalizing a projection containing an inference variable
// which cannot be named by the `root_vid` can result in ambiguity.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
error[E0284]: type annotations needed: cannot satisfy `<<Leaf as WithAssoc<_>>::Assoc as Id>::Assoc == <<Leaf as WithAssoc<_>>::Assoc as Id>::Assoc`
error[E0284]: type annotations needed: cannot satisfy `<<Rigid as IdHigherRankedBound>::Assoc as WithAssoc<<Wrapper<Leaf> as Id>::Assoc>>::Assoc normalizes-to <<Leaf as WithAssoc<_>>::Assoc as Id>::Assoc`
--> $DIR/generalize-proj-new-universe-index-2.rs:74:5
|
LL | bound::<<Rigid as IdHigherRankedBound>::Assoc, <Wrapper<Leaf> as Id>::Assoc, _>()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot satisfy `<<Leaf as WithAssoc<_>>::Assoc as Id>::Assoc == <<Leaf as WithAssoc<_>>::Assoc as Id>::Assoc`
|
note: required by a bound in `bound`
--> $DIR/generalize-proj-new-universe-index-2.rs:69:21
|
LL | fn bound<T: ?Sized, U: ?Sized, V: ?Sized>()
| ----- required by a bound in this function
LL | where
LL | T: WithAssoc<U, Assoc = V>,
| ^^^^^^^^^ required by this bound in `bound`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot satisfy `<<Rigid as IdHigherRankedBound>::Assoc as WithAssoc<<Wrapper<Leaf> as Id>::Assoc>>::Assoc normalizes-to <<Leaf as WithAssoc<_>>::Assoc as Id>::Assoc`

error: aborting due to 1 previous error

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
//~ ERROR overflow
// compile-flags: -Ztrait-solver=next

trait Foo1 {
Expand All @@ -15,6 +14,7 @@ fn needs_bar<S: Bar>() {}
fn test<T: Foo1<Assoc1 = <T as Foo2>::Assoc2> + Foo2<Assoc2 = <T as Foo1>::Assoc1>>() {
needs_bar::<T::Assoc1>();
//~^ ERROR overflow evaluating the requirement `<T as Foo1>::Assoc1: Bar`
//~| ERROR overflow evaluating the requirement `<T as Foo2>::Assoc2`
}

fn main() {}
Loading