Show env normalization differences under two solvers

[adwinwhite]

First step for param_env normalization future compat warning. zulip thread

I didn't put the check directly in param_env query because normalize_param_env_or_error has some adjustments on the predicates: elaboration and outlive predicates manipulation. I'd have to replicate these in param_env to compare normalized predicates properly.
The downside of putting the check inside normalize_param_env_or_error is that it's used in more than param_env query. It's also used in compare_impl_item and several other places. I'm not sure if this is desired.

I didn't bless tests since the hard error will be changed to lint later.
Blessed tests to demonstrate the changes.

And there's one test about const generics I don't know how to fix.
Canonicalizer for the next solver will replace ParamConst with PlaceholderConst, but it still uses the same try_evaluate_const internally and process_obligation doesn't like PlaceholderConst.

r? @lcnr

[rustbot]

Some changes occurred to the core trait solver

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

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[lcnr]

need to fully resolve the predicates before comparing them. This is causing the error in std. Also just generally move this further to the end of this function?

I think doing it here isn't ideally, but should be good enough for a crater run

[adwinwhite]

std compiles fine after resolving predicates normalized by the old solver.
It turns out that stderr changes of some tests are noise. They're restored to the original state.

[adwinwhite]

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[adwinwhite]

Should projection alias in param env imply that the trait predicate holds for the next solver?
I add manual trait clause for these three specialization tests to avoid unnecessary lint triggering.

[adwinwhite]

If we resolve regions for the next solver, these two tests will trigger ICEs. They're the same tests mentioned in 166.

• tests/ui/higher-ranked/trait-bounds/issue-102899.rs
• tests/ui/higher-ranked/trait-bounds/issue-100689.rs

The cause is that we add constraints that contains placeholders to infcx's constraints collector and resolve_regions can't handle that.
We replace bounds with placeholders and restore them after trait solving in deeply_normalize, but the restoration doesn't apply to region constraints added to infcx.

Now the only ui test broken should be the one in PR description. Sorry for not running all tests faithfully before.

[adwinwhite]

The next solver normalizes free alias to opaque while the old solver doesn't.

[lcnr]

this is not a diverging between the trait solvers, but instead deeply normalizing with the new solver not checking whether predicates may get normalized.

see

rust/compiler/rustc_trait_selection/src/traits/normalize.rs

Lines 484 to 490 in 66bc5a4

	fn fold_predicate(&mut self, p: ty::Predicate<'tcx>) -> ty::Predicate<'tcx> {
	if p.allow_normalization() && needs_normalization(self.selcx.infcx, &p) {
	p.super_fold_with(self)
	} else {
	p
	}
	}

which does not exist in the new solver deeply normalize as we've never deeply normalized predicates before

[adwinwhite]

Should we add a try_fold_predicate with this check to the next solver version as well?

[lcnr]

yes

[adwinwhite]

The next solver normalizes projection alias to opaque while the old solver doesn't.

[lcnr]

If we resolve regions for the next solver, these two tests will trigger ICEs. They're the same tests mentioned in 166.

* tests/ui/higher-ranked/trait-bounds/issue-102899.rs

* tests/ui/higher-ranked/trait-bounds/issue-100689.rs

The cause is that we add constraints that contains placeholders to infcx's constraints collector and resolve_regions can't handle that. We replace bounds with placeholders and restore them after trait solving in deeply_normalize, but the restoration doesn't apply to region constraints added to infcx.

Now the only ui test broken should be the one in PR description. Sorry for not running all tests faithfully before.

this should similarly affect the old solver except that it does not register outlives constraints if considering_regions is set to false :<

I feel like we can probably just disable that ICE/assert for now, idk where exactly that happens and didn't look into it before writing this comment

[adwinwhite]

this should similarly affect the old solver except that it does not register outlives constraints if considering_regions is set to false :<

yeah, the next solver doesn't seem to check infcx.considering_regions as I ripgrepped.
Should we check it in some region constraint registering function of infcx or the next solver?

I feel like we can probably just disable that ICE/assert for now, idk where exactly that happens and didn't look into it before writing this comment

Got it. I'll ignore the errors of resolve_regions for now.

[adwinwhite]

About ICE in this test.

I understand that the generic_const_exprs is not supported by the next solver for now.
The problem is that this test uses generic_const_exprs features in dependency crate so we can't skip our lint before hand.
If we can know whether dependencies use generic_const_exprs and disable out lint, the lint's usefulness is damaged?

Now that we accept we will run into generic const in normalization with the next solver, there're two choices:

• detect generic param and return the unevaluated const back. (The old normalization does this)
• return error as the normalization does fail. (My impression is that normalization with the next solver is less forgiving)

[adwinwhite]

@rustbot ready

[lcnr]

@bors try

[rust-bors]

☀️ Try build successful (CI)
Build commit: c8085cf (c8085cfed34c45021fc8f66d073f6394b3e35431, parent: 217cb73577ed6f30a2005dd75bab01d23ec4cd60)

[lcnr]

@craterbot check

[craterbot]

👌 Experiment pr-148939 created and queued.
🤖 Automatically detected try build c8085cf
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more