Skip to content
Open
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
9 changes: 8 additions & 1 deletion compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,14 @@ fn compare_method_predicate_entailment<'tcx>(

let normalize_cause = traits::ObligationCause::misc(impl_m_span, impl_m_def_id);
let param_env = ty::ParamEnv::new(tcx.mk_clauses(&hybrid_preds));
let param_env = traits::normalize_param_env_or_error(tcx, param_env, normalize_cause);
// FIXME(-Zhigher-ranked-assumptions): lazy normalization may postpone region constraints to
// an infcx that checks regions. Deeply normalize the param env in the next solver as well.
// cc trait-system-refactor-initiative/issues/166.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// cc trait-system-refactor-initiative/issues/166.
// FIXME(-Zhigher-ranked-assumptions): The `hybrid_preds`
// should be well-formed. However, using them may result in
// region errors as we currently don't track placeholder
// assumptions.
//
// To avoid being backwards incompatible with the old solver,
// we also eagerly normalize the where-bounds in the new solver
// here while ignoring region constraints. This means we can then
// use where-bounds whose normalization results in placeholder
// errors further down without getting any errors.
//
// It should be sound to do so as the only region errors here
// should be due to missing implied bounds.
//
// cc trait-system-refactor-initiative/issues/166.

elaborated a bit here

let param_env = if tcx.next_trait_solver_globally() {
traits::deeply_normalize_param_env_or_error(tcx, param_env, normalize_cause)
} else {
traits::normalize_param_env_or_error(tcx, param_env, normalize_cause)
};
debug!(caller_bounds=?param_env.caller_bounds());

let infcx = &tcx.infer_ctxt().build(TypingMode::non_body_analysis());
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_trait_selection/src/solve/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ where
if ty.has_escaping_bound_vars() {
let (ty, mapped_regions, mapped_types, mapped_consts) =
BoundVarReplacer::replace_bound_vars(infcx, &mut self.universes, ty);
// FIXME: Temporary placeholders may get added to infcx's region constraints/obligations,
// which can cause problem for `resolve_regions`.
Comment on lines +201 to +202
Copy link
Contributor

Choose a reason for hiding this comment

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

what do you mean here? I agree that we may get constraints involving placeholders from this, but that's fine™

We have the same behavior when relating binders and what not. The way we handle such placeholder constraints may change going forward and ideally we'd eagerly handle all of them when "exiting" the binder again, but for now "leaking" these constraints is normal

let result =
ensure_sufficient_stack(|| self.normalize_alias_term(ty.into()))?.expect_type();
Ok(PlaceholderReplacer::replace_placeholders(
Expand Down
112 changes: 112 additions & 0 deletions compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,58 @@ fn do_normalize_predicates<'tcx>(
}
}

// A temporary hack. Do not use this.
// See `deeply_normalize_param_env_or_error`.
#[instrument(level = "debug", skip(tcx, elaborated_env))]
fn do_deeply_normalize_predicates<'tcx>(
tcx: TyCtxt<'tcx>,
cause: ObligationCause<'tcx>,
elaborated_env: ty::ParamEnv<'tcx>,
predicates: Vec<ty::Clause<'tcx>>,
) -> Result<Vec<ty::Clause<'tcx>>, ErrorGuaranteed> {
let span = cause.span;
let infcx = tcx
.infer_ctxt()
.with_next_trait_solver(true)
.ignoring_regions()
.build(TypingMode::non_body_analysis());
let predicates = match crate::solve::deeply_normalize::<_, FulfillmentError<'tcx>>(
infcx.at(&cause, elaborated_env),
predicates,
) {
Ok(predicates) => predicates,
Err(errors) => {
let reported = infcx.err_ctxt().report_fulfillment_errors(errors);
return Err(reported);
}
};

debug!("do_normalize_predicates: normalized predicates = {:?}", predicates);

// The next solver doesn't ignore region constraints so we do it manually.
drop(infcx.take_registered_region_obligations());
drop(infcx.take_registered_region_assumptions());
drop(infcx.take_and_reset_region_constraints());
Comment on lines +349 to +351
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, this feels... really bad xd

like, that's kind of the point of what we're doing here, but still

Does not dropping these things and instead not emitting a span_delayed_bug if there are errors also work? And also add an explicit comment referencing the FIXME from the function docs

let errors = infcx.resolve_regions(cause.body_id, elaborated_env, []);
if !errors.is_empty() {
tcx.dcx().span_delayed_bug(
span,
format!("failed region resolution while normalizing {elaborated_env:?}: {errors:?}"),
);
}

match infcx.fully_resolve(predicates) {
Ok(predicates) => Ok(predicates),
Err(fixup_err) => {
span_bug!(
span,
"inference variables in normalized parameter environment: {}",
fixup_err
)
}
}
}

// FIXME: this is gonna need to be removed ...
/// Normalizes the parameter environment, reporting errors if they occur.
#[instrument(level = "debug", skip(tcx))]
Expand Down Expand Up @@ -477,6 +529,66 @@ pub fn normalize_param_env_or_error<'tcx>(
ty::ParamEnv::new(tcx.mk_clauses(&predicates))
}

// FIXME(-Zhigher-ranked-assumptions): this is a hack to walk around the fact that we don't support
// placeholder assumptions right now.
// We should remove this once we have proper support for implied bounds on binders.
/// Deeply normalize the param env using the next solver.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Deeply normalize the param env using the next solver.
/// Deeply normalize the param env using the next solver ignoring
/// region errors.
///
/// FIXME(-Zhigher-ranked-assumptions): this is a hack to work around
/// the fact that we don't support placeholder assumptions right now
/// and is necessary for `compare_method_predicate_entailment`, see the
/// use of this function for more info. We should remove this once we
/// have proper support for implied bounds on binders.

elaborated a bit and also want the FIXME to be part of the doc comment. The fact that this function is somewhat scuffed and shouldn't be used should be visible in the docs

#[instrument(level = "debug", skip(tcx))]
pub fn deeply_normalize_param_env_or_error<'tcx>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub fn deeply_normalize_param_env_or_error<'tcx>(
pub fn deeply_normalize_param_env_ignoring_regions<'tcx>(

tcx: TyCtxt<'tcx>,
unnormalized_env: ty::ParamEnv<'tcx>,
cause: ObligationCause<'tcx>,
) -> ty::ParamEnv<'tcx> {
let mut predicates: Vec<_> =
util::elaborate(tcx, unnormalized_env.caller_bounds().into_iter()).collect();

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

let elaborated_env = ty::ParamEnv::new(tcx.mk_clauses(&predicates));
Copy link
Contributor

Choose a reason for hiding this comment

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

don't think we have to create a new param_env at all here 🤔 unsure why we do it in the old solver...

wanna update the new solver to just use the unnormalized_env instead?

if !elaborated_env.has_aliases() {
return elaborated_env;
}

let outlives_predicates: Vec<_> = predicates
.extract_if(.., |predicate| {
matches!(predicate.kind().skip_binder(), ty::ClauseKind::TypeOutlives(..))
})
.collect();

debug!(
"normalize_param_env_or_error: predicates=(non-outlives={:?}, outlives={:?})",
predicates, outlives_predicates
);
let Ok(non_outlives_predicates) =
do_deeply_normalize_predicates(tcx, cause.clone(), elaborated_env, predicates)
else {
// An unnormalized env is better than nothing.
debug!("normalize_param_env_or_error: errored resolving non-outlives predicates");
return elaborated_env;
};

debug!("normalize_param_env_or_error: non-outlives predicates={:?}", non_outlives_predicates);

// Not sure whether it is better to include the unnormalized TypeOutlives predicates
// here. I believe they should not matter, because we are ignoring TypeOutlives param-env
// predicates here anyway. Keeping them here anyway because it seems safer.
let outlives_env = non_outlives_predicates.iter().chain(&outlives_predicates).cloned();
let outlives_env = ty::ParamEnv::new(tcx.mk_clauses_from_iter(outlives_env));
let Ok(outlives_predicates) =
do_deeply_normalize_predicates(tcx, cause, outlives_env, outlives_predicates)
else {
// An unnormalized env is better than nothing.
debug!("normalize_param_env_or_error: errored resolving outlives predicates");
return elaborated_env;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

don't need to split outlives vs non-outlives predicate norm in the new solver. This only matters for the old one

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that way you also don't need a separate do_deeply_normalize_predicates function as you only call it once

debug!("normalize_param_env_or_error: outlives predicates={:?}", outlives_predicates);

let mut predicates = non_outlives_predicates;
predicates.extend(outlives_predicates);
debug!("normalize_param_env_or_error: final predicates={:?}", predicates);
ty::ParamEnv::new(tcx.mk_clauses(&predicates))
}

#[derive(Debug)]
pub enum EvaluateConstErr {
/// The constant being evaluated was either a generic parameter or inference variable, *or*,
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/higher-ranked/trait-bounds/issue-100689.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
//@ check-pass
//@ revisions: old next
//@[next] compile-flags: -Znext-solver

struct Foo<'a> {
foo: &'a mut usize,
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/higher-ranked/trait-bounds/issue-102899.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
//@ check-pass
//@ revisions: old next
//@[next] compile-flags: -Znext-solver

pub trait BufferTrait<'buffer> {
type Subset<'channel>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//@ check-pass
//@ compile-flags: -Znext-solver

// See trait-system-refactor-initiative/issues/166.
// The old solver doesn't check normalization constraints in `compare_impl_item`.
// The new solver performs lazy normalization so those region constraints may get postponed to
// an infcx that considers regions.
trait Trait {
type Assoc<'a>
where
Self: 'a;
}
impl<'b> Trait for &'b u32 {
type Assoc<'a> = &'a u32
where
Self: 'a;
}

trait Bound<T> {}
trait Entailment<T: Trait> {
fn method()
where
Self: for<'a> Bound<<T as Trait>::Assoc<'a>>;
}

impl<'b, T> Entailment<&'b u32> for T {
// Instantiates trait where-clauses with `&'b u32` and then normalizes
// `T: for<'a> Bound<<&'b u32 as Trait>::Assoc<'a>>` in a separate infcx
// without checking region constraints.
//
// It normalizes to `T: Bound<&'a u32>`, dropping the `&'b u32: 'a` constraint.
fn method()
where
Self: for<'a> Bound<&'a u32>
{}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
error[E0275]: overflow evaluating the requirement `<() as A<T>>::Assoc == _`
--> $DIR/normalize-param-env-2.rs:22:5
|
LL | / fn f()
LL | | where
LL | | Self::Assoc: A<T>,
| |__________________________^

error[E0275]: overflow evaluating the requirement `<() as A<T>>::Assoc: A<T>`
--> $DIR/normalize-param-env-2.rs:24:22
|
Expand Down Expand Up @@ -46,6 +54,6 @@ LL | where
LL | Self::Assoc: A<T>,
| ^^^^ required by this bound in `A::f`

error: aborting due to 5 previous errors
error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0275`.
Loading