Skip to content

move super_relate_consts hack to normalize_param_env_or_error #111623

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
May 31, 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
11 changes: 0 additions & 11 deletions compiler/rustc_middle/src/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -589,17 +589,6 @@ pub fn structurally_relate_consts<'tcx, R: TypeRelation<'tcx>>(
debug!("{}.structurally_relate_consts(a = {:?}, b = {:?})", relation.tag(), a, b);
let tcx = relation.tcx();

// HACK(const_generics): We still need to eagerly evaluate consts when
// relating them because during `normalize_param_env_or_error`,
// we may relate an evaluated constant in a obligation against
// an unnormalized (i.e. unevaluated) const in the param-env.
// FIXME(generic_const_exprs): Once we always lazily unify unevaluated constants
// these `eval` calls can be removed.
if !tcx.features().generic_const_exprs {
a = a.eval(tcx, relation.param_env());
b = b.eval(tcx, relation.param_env());
}

if tcx.features().generic_const_exprs {
a = tcx.expand_abstract_consts(a);
b = tcx.expand_abstract_consts(b);
Expand Down
60 changes: 57 additions & 3 deletions compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use rustc_errors::ErrorGuaranteed;
use rustc_middle::query::Providers;
use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::visit::{TypeVisitable, TypeVisitableExt};
use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, TypeSuperVisitable};
use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, TypeFolder, TypeSuperVisitable};
use rustc_middle::ty::{InternalSubsts, SubstsRef};
use rustc_span::def_id::DefId;
use rustc_span::Span;
Expand Down Expand Up @@ -272,8 +272,62 @@ pub fn normalize_param_env_or_error<'tcx>(
// parameter environments once for every fn as it goes,
// and errors will get reported then; so outside of type inference we
// can be sure that no errors should occur.
let mut predicates: Vec<_> =
util::elaborate(tcx, unnormalized_env.caller_bounds().into_iter()).collect();
let mut predicates: Vec<_> = util::elaborate(
tcx,
unnormalized_env.caller_bounds().into_iter().map(|predicate| {
if tcx.features().generic_const_exprs {
return predicate;
}

struct ConstNormalizer<'tcx>(TyCtxt<'tcx>);

impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ConstNormalizer<'tcx> {
fn interner(&self) -> TyCtxt<'tcx> {
self.0
}

fn fold_const(&mut self, c: ty::Const<'tcx>) -> ty::Const<'tcx> {
// While it is pretty sus to be evaluating things with an empty param env, it
// should actually be okay since without `feature(generic_const_exprs)` the only
// const arguments that have a non-empty param env are array repeat counts. These
// do not appear in the type system though.
c.eval(self.0, ty::ParamEnv::empty())
}
}

// This whole normalization step is a hack to work around the fact that
// `normalize_param_env_or_error` is fundamentally broken from using an
// unnormalized param env with a trait solver that expects the param env
// to be normalized.
//
// When normalizing the param env we can end up evaluating obligations
// that have been normalized but can only be proven via a where clause
// which is still in its unnormalized form. example:
//
// Attempting to prove `T: Trait<<u8 as Identity>::Assoc>` in a param env
// with a `T: Trait<<u8 as Identity>::Assoc>` where clause will fail because
// we first normalize obligations before proving them so we end up proving
// `T: Trait<u8>`. Since lazy normalization is not implemented equating `u8`
// with `<u8 as Identity>::Assoc` fails outright so we incorrectly believe that
// we cannot prove `T: Trait<u8>`.
//
// The same thing is true for const generics- attempting to prove
// `T: Trait<ConstKind::Unevaluated(...)>` with the same thing as a where clauses
// will fail. After normalization we may be attempting to prove `T: Trait<4>` with
// the unnormalized where clause `T: Trait<ConstKind::Unevaluated(...)>`. In order
// for the obligation to hold `4` must be equal to `ConstKind::Unevaluated(...)`
// but as we do not have lazy norm implemented, equating the two consts fails outright.
//
// Ideally we would not normalize consts here at all but it is required for backwards
// compatibility. Eventually when lazy norm is implemented this can just be removed.
// We do not normalize types here as there is no backwards compatibility requirement
// for us to do so.
//
// FIXME(-Ztrait-solver=next): remove this hack since we have deferred projection equality
predicate.fold_with(&mut ConstNormalizer(tcx))
}),
)
.collect();

debug!("normalize_param_env_or_error: elaborated-predicates={:?}", predicates);

Expand Down
26 changes: 24 additions & 2 deletions tests/ui/traits/new-solver/structural-resolve-field.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,35 @@
// compile-flags: -Ztrait-solver=next
// check-pass

#[derive(Default)]
struct Foo {
x: i32,
}

impl MyDefault for Foo {
fn my_default() -> Self {
Self {
x: 0,
}
}
}

trait MyDefault {
fn my_default() -> Self;
}

impl MyDefault for [Foo; 0] {
fn my_default() -> Self {
[]
}
}
impl MyDefault for [Foo; 1] {
fn my_default() -> Self {
[Foo::my_default(); 1]
}
}

fn main() {
let mut xs = <[Foo; 1]>::default();
Copy link
Member Author

@BoxyUwU BoxyUwU May 26, 2023

Choose a reason for hiding this comment

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

attemtping to prove [Foo; 1]: Default breaks under the new solver with this PR because the impl in std is [Foo; AnonConst]: Default which we equate with [Foo; 1]: Default. New solver does not normalize constants so AnonConst does not unify with 1 even though evaluating the anon const would result in 1. This test was accidentally relying on the super_relate_consts hack which is a great example of why we need this PR lol

This PR is fine to land even with this, new solver is completely experimental and needs to have const normalization implemented at some point anyway.

let mut xs = <[Foo; 1]>::my_default();
xs[0].x = 1;
(&mut xs[0]).x = 2;
}