Deeply normalize obligations in BestObligation folder

[compiler-errors]

Built on #139513.

This establishes a somewhat rough invariant that the Obligation's predicate is always deeply normalized in the folder; when we construct a new obligation we normalize it.

Putting this up for discussion since it does affect some goals.

r? lcnr

[rustbot]

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

[compiler-errors]

AliasRelate goals are negatively affected by this PR, which should make sense b/c we're normalizing the LHS and RHS; we probably should special-case ProjectionPredicates, since handling AliasRelate predicates is already kinda weird:

rust/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs

Lines 1422 to 1465 in d4f880f

	ty::PredicateKind::AliasRelate(lhs, rhs, _) => {
	let derive_better_type_error =
	|alias_term: ty::AliasTerm<'tcx>, expected_term: ty::Term<'tcx>| {
	let ocx = ObligationCtxt::new(self);
	let Ok(normalized_term) = ocx.structurally_normalize_term(
	&ObligationCause::dummy(),
	obligation.param_env,
	alias_term.to_term(self.tcx),
	) else {
	return None;
	};
	if let Err(terr) = ocx.eq(
	&ObligationCause::dummy(),
	obligation.param_env,
	expected_term,
	normalized_term,
	) {
	Some((terr, self.resolve_vars_if_possible(normalized_term)))
	} else {
	None
	}
	};
	if let Some(lhs) = lhs.to_alias_term()
	&& let Some((better_type_err, expected_term)) =
	derive_better_type_error(lhs, rhs)
	{
	(
	Some((lhs, self.resolve_vars_if_possible(expected_term), rhs)),
	better_type_err,
	)
	} else if let Some(rhs) = rhs.to_alias_term()
	&& let Some((better_type_err, expected_term)) =
	derive_better_type_error(rhs, lhs)
	{
	(
	Some((rhs, self.resolve_vars_if_possible(expected_term), lhs)),
	better_type_err,
	)
	} else {
	(None, error.err)
	}

.

[lcnr]

hmm yeah, I feel like we should do the "alias-relate -> normalization failure" transformation in the BestObligation visitor instead maybe 🤔

This is definitely worse... also in the next-solver/async.rs#fail test

[compiler-errors]

I'd rather defer this to a different PR. Right now the regression seems like a logical consequence of this implementation, and I'll play around with a fix for the AliasRelate regression separately.

[compiler-errors]

commit_if_ok b/c we shouldn't apply inference/opaque side-effects if the nested goals don't hold.

[bors]

☔ The latest upstream changes (presumably #139555) made this pull request unmergeable. Please resolve the merge conflicts.

[lcnr]

so the idea is to deeply normalize the resulting obligations, but not deeply normalizing the goal we use to recur?

In that case, would it be easier to add a single deeply normalize call on the result of the visitor?

[compiler-errors]

In that case, would it be easier to add a single deeply normalize call on the result of the visitor?

lol, I thought we didn't fold ObligationCauseCodes, but it seems like we do. Yeah, that's simpler then.

[lcnr]

@bors r+ rollup

[bors]

📌 Commit 4192cb6 has been approved by lcnr

It is now in the queue for this repository.

[compiler-errors]

@bors r- #139513 (comment)

[bors]

☔ The latest upstream changes (presumably #139595) made this pull request unmergeable. Please resolve the merge conflicts.

[compiler-errors]

@bors r=lcnr

[bors]

📌 Commit decd7ec has been approved by lcnr

It is now in the queue for this repository.