Deeply normalize param env in `compare_impl_item` if using the next solver #149345

adwinwhite · 2025-11-26T08:18:57Z

Fixes rust-lang/trait-system-refactor-initiative#166.

Duplicated the normalize_param_env_or_error function to force deep normalization for compare_impl_item.

r? @lcnr

rustbot · 2025-11-26T08:19:01Z

Some changes occurred to the core trait solver

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

lcnr

thanks for working on this!

View changes since this review

lcnr · 2025-11-26T08:39:16Z

compiler/rustc_trait_selection/src/traits/mod.rs

+        // An unnormalized env is better than nothing.
+        debug!("normalize_param_env_or_error: errored resolving outlives predicates");
+        return elaborated_env;
+    };


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

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

lcnr · 2025-11-26T08:47:59Z

compiler/rustc_trait_selection/src/traits/mod.rs

+    drop(infcx.take_registered_region_obligations());
+    drop(infcx.take_registered_region_assumptions());
+    drop(infcx.take_and_reset_region_constraints());


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

lcnr · 2025-11-26T08:51:09Z

compiler/rustc_trait_selection/src/traits/mod.rs

+
+    debug!("normalize_param_env_or_error: elaborated-predicates={:?}", predicates);
+
+    let elaborated_env = ty::ParamEnv::new(tcx.mk_clauses(&predicates));


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?

Because we need to use the elaborated_env as the param_env of normalization/resolving and the return value in the error path?
unnormalized_env doesn't contain those elaborated predicates. And using it causes some regressions in my testing.

oh 😅 I am blind xd

lcnr · 2025-11-26T08:52:03Z

compiler/rustc_trait_selection/src/traits/mod.rs

+// 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.


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

lcnr · 2025-11-26T08:53:54Z

compiler/rustc_trait_selection/src/solve/normalize.rs

+            // FIXME: Temporary placeholders may get added to infcx's region constraints/obligations,
+            // which can cause problem for `resolve_regions`.


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

lcnr · 2025-11-26T08:56:53Z

compiler/rustc_trait_selection/src/traits/mod.rs

+// We should remove this once we have proper support for implied bounds on binders.
+/// Deeply normalize the param env using the next solver.
+#[instrument(level = "debug", skip(tcx))]
+pub fn deeply_normalize_param_env_or_error<'tcx>(


Suggested change

pub fn deeply_normalize_param_env_or_error<'tcx>(

pub fn deeply_normalize_param_env_ignoring_regions<'tcx>(

lcnr · 2025-11-26T09:02:07Z

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

-    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.


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

lcnr · 2025-11-27T10:09:44Z

awesome, thanks

@bors r+ rollup

bors · 2025-11-27T10:09:47Z

📌 Commit 133d520 has been approved by lcnr

It is now in the queue for this repository.

Rollup of 6 pull requests Successful merges: - #148256 (remove support for `typeof`) - #148589 (Rename `DropGuard::into_inner` to `DropGuard::dismiss`) - #149001 (Fix false positive of "multiple different versions of crate X in the dependency graph") - #149334 (fix ICE: rustdoc: const parameter types cannot be generic #149288) - #149345 (Deeply normalize param env in `compare_impl_item` if using the next solver) - #149367 (Tidying up UI tests [4/N]) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #149345 - adwinwhite:next-166, r=lcnr Deeply normalize param env in `compare_impl_item` if using the next solver Fixes rust-lang/trait-system-refactor-initiative#166. Duplicated the `normalize_param_env_or_error` function to force deep normalization for `compare_impl_item`. r? `@lcnr`

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Nov 26, 2025

rustbot assigned lcnr Nov 26, 2025

This comment has been minimized.

Sign in to view

adwinwhite force-pushed the next-166 branch from ed48994 to 1fa0851 Compare November 26, 2025 08:36

lcnr reviewed Nov 26, 2025

View reviewed changes

deeply normalize param env in compare_impl_item

133d520

adwinwhite force-pushed the next-166 branch from 1fa0851 to 133d520 Compare November 27, 2025 08:35

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 27, 2025

matthiaskrgr mentioned this pull request Nov 27, 2025

Rollup of 6 pull requests #149387

Merged

bors merged commit a8cf0c7 into rust-lang:main Nov 27, 2025
11 checks passed

rustbot added this to the 1.93.0 milestone Nov 27, 2025

lcnr mentioned this pull request Nov 28, 2025

old solver doesn't check normalization constraints in compare_impl_item rust-lang/trait-system-refactor-initiative#166

Closed


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

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

-/// 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.

		// FIXME: Temporary placeholders may get added to infcx's region constraints/obligations,
		// which can cause problem for `resolve_regions`.

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

-    // 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.

Deeply normalize param env in compare_impl_item if using the next solver #149345

Deeply normalize param env in compare_impl_item if using the next solver #149345

Conversation

adwinwhite commented Nov 26, 2025

Uh oh!

rustbot commented Nov 26, 2025

Uh oh!

This comment has been minimized.

lcnr left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adwinwhite Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr commented Nov 27, 2025

Uh oh!

bors commented Nov 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Deeply normalize param env in `compare_impl_item` if using the next solver #149345

Deeply normalize param env in `compare_impl_item` if using the next solver #149345

lcnr left a comment •

edited by rustbot

Loading

adwinwhite Nov 27, 2025 •

edited

Loading