Introduce `Unnormalized` wrapper by adwinwhite · Pull Request #155083 · rust-lang/rust

adwinwhite · 2026-04-10T07:23:20Z

This is the first step of the eager normalization series.

This PR introduce an Unnormalized wrapper and make most normalization routines consume it. The purpose is to make normalization explicit.
This PR contains no behavior change.

API changes are in the first two commit.
There're some normalization routines left untouched:

normalize in the type checker of borrowck: better do it together with field.ty() returning Unnormalized.
normalize_with_depth: only used inside the old solver. Can be done later.
query_normalize: rarely used.
misc local normalization helpers.

The compiler errors are mostly fixed via ast-grep, with exceptions handled manually.

adwinwhite · 2026-04-10T07:24:17Z

@bors try @rust-timer queue

Introduce `Unnormalized` wrapper

adwinwhite · 2026-04-10T07:38:24Z

@bors try cancel

rust-bors · 2026-04-10T07:38:29Z

Try build cancelled. Cancelled workflows:

https://github.com/rust-lang/rust/actions/runs/24231666610

adwinwhite · 2026-04-10T09:31:13Z

@bors try @rust-timer queue

Introduce `Unnormalized` wrapper

rust-bors · 2026-04-10T11:43:50Z

☀️ Try build successful (CI)
Build commit: 10d0ef4 (10d0ef4e7995ca1d162f402be9f0b39b15d7a78a, parent: 7659cec4ed1457bc0d1f636127e66e8fe5008928)

rust-timer · 2026-04-10T12:24:19Z

Finished benchmarking commit (10d0ef4): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary -2.4%, secondary -6.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.4%	[-2.4%, -2.4%]	1
Improvements ✅ (secondary)	-6.4%	[-6.4%, -6.4%]	1
All ❌✅ (primary)	-2.4%	[-2.4%, -2.4%]	1

Cycles

Results (secondary -0.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.4%	[2.1%, 2.7%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.7%	[-3.8%, -3.6%]	2
All ❌✅ (primary)	-	-	0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 492.281s -> 489.321s (-0.60%)
Artifact size: 395.57 MiB -> 395.56 MiB (-0.00%)

adwinwhite · 2026-04-10T13:41:30Z

r? @lcnr

lcnr · 2026-04-15T14:22:45Z

    {
-        self.normalize_with_category(value, location, ConstraintCategory::Boring)
+        self.normalize_with_category(
+            Unnormalized::new_wip(value),


not propagating this one out?

View changes since the review

lcnr · 2026-04-15T14:23:24Z

    where
        T: type_op::normalize::Normalizable<'tcx> + fmt::Display + Copy + 'tcx,
    {
+        let value = value.inside_norm();


Suggested change

let value = value.inside_norm();

let value = value.skip_norm_wip();

would want us to propagate the Unnormalized into the type op and only discard it when actually normalizing

View changes since the review

lcnr · 2026-04-15T14:36:58Z

+                let unnormalized_ty =
+                    tcx.type_of(static_def_id).instantiate_identity().skip_norm_wip();
                let normalized_ty = self.normalize(unnormalized_ty, locations);


hmm, want to change TypeChecker::normalize to also take Unnormalized

View changes since the review

lcnr · 2026-04-15T14:52:09Z

    // restrictive than the trait's method (and the impl itself).
-    let impl_m_own_bounds = impl_m_predicates.instantiate_own_identity();
+    let impl_m_own_bounds =
+        impl_m_predicates.instantiate_own_identity().map(|(c, s)| (c.skip_norm_wip(), s));


that skip_norm_wip seems overeager given that we do actually just do ocx.normalize on that predicate right afer

View changes since the review

lcnr · 2026-04-15T14:52:19Z

+        for (const_condition, span) in tcx
+            .const_conditions(impl_m.def_id)
+            .instantiate_own_identity()
+            .map(|(c, s)| (c.skip_norm_wip(), s))


same here

View changes since the review

lcnr · 2026-04-15T14:59:47Z

                    continue;
                }
-                if !tcx.is_dyn_compatible(trait_ref.def_id) {
+                if !tcx.is_dyn_compatible(trait_ref.probe(|v| v.def_id)) {


probe is an overloaded name in the type system 🤔 maybe .extract

I would not use this to get a def_id as I want that to have a separate function fn def_id. I don't like this as a final state here and unlike skip_norm_wip, it's harder to grep for

View changes since the review

yeah... I don't really like probe. Can you always do skip_norm_wf or skip_normalization in cases where we we know that we don't want to normalize

lcnr · 2026-04-15T15:05:55Z

-        let (normalized_ty, obligations) =
-            self.structurally_normalize_ty(Ty::new_projection(tcx, trait_target_def_id, [ty]))?;
+        let (normalized_ty, obligations) = self.structurally_normalize_ty(
+            Unnormalized::new_wip(Ty::new_projection(tcx, trait_target_def_id, [ty])),


Suggested change

Unnormalized::new_wip(Ty::new_projection(tcx, trait_target_def_id, [ty])),

Unnormalized::new(Ty::new_projection(tcx, trait_target_def_id, [ty])),

very intentionally unnormalized :3

View changes since the review

lcnr · 2026-04-15T15:06:33Z

                &ObligationCause::dummy(),
                param_env,
-                Ty::new_projection_from_args(infcx.tcx, assoc_item_def_id, args),
+                Unnormalized::new_wip(Ty::new_projection_from_args(


Suggested change

Unnormalized::new_wip(Ty::new_projection_from_args(

Unnormalized::new(Ty::new_projection_from_args(

also very intentionally unnormalized

View changes since the review

lcnr · 2026-04-15T15:17:20Z

-                    cx.typing_env(),
-                    Ty::new_projection_from_args(cx.tcx, target_id, cx.tcx.mk_args(&[GenericArg::from(indexed_ty)])),
-                )
+                && let Ok(deref_ty) = cx.tcx.try_normalize_erasing_regions(cx.typing_env(), Unnormalized::new_wip(Ty::new_projection_from_args(cx.tcx, target_id, cx.tcx.mk_args(&[GenericArg::from(indexed_ty)]))))


can you look through your changes to clippy and avoid breaking formatting as much in your diff? 😅 sorry

View changes since the review

lcnr · 2026-04-15T15:18:53Z

            // If is a Future
-            && preds
-                .iter_instantiated_copied(cx.tcx, args)
+            && preds.iter_instantiated_copied(cx.tcx, args).map(Unnormalized::skip_normalization)


what are the places that still use skip_normalization instead of skip_norm_wip?

View changes since the review

lcnr · 2026-04-15T15:23:33Z

+        f(&self.value)
+    }
+
+    pub fn map_inner<F, U>(self, f: F) -> Unnormalized<I, U>


just call this map

View changes since the review

lcnr · 2026-04-15T15:24:58Z

+    /// FIXME: Should I just use `skip_normalization`?
+    /// Kinda weird to call that insides normalization methods.
+    pub fn inside_norm(self) -> T {
+        self.value
+    }


I would call that one skip_normalization and then it should be clear from context why we are 🤔

could imagine being more specific about why we're intentianally not normalizing, but I think for now it's fine for that to be (poorly) implied by the context

View changes since the review

lcnr · 2026-04-15T15:32:55Z

-    pub fn instantiate_identity(self) -> T {
-        self.value
+    pub fn instantiate_identity(self) -> Unnormalized<I, T> {
+        Unnormalized::new(self.value)


Suggested change

Unnormalized::new(self.value)

// FIXME(#155345): In case the bound value was already normalized, this

// is unnecessary. We may want to track explicitly whether `EarlyBinder`

// contains something that has been normalized already.

//

// This is annoying, as e.g. `type_of` for opaque types is normalized,

// while `type_of` for free type aliases is not.

Unnormalized::new(self.value)

View changes since the review

lcnr · 2026-04-15T15:35:34Z

+                    .map(Unnormalized::skip_norm_wip)
+                    .filter_map(ty::Clause::as_trait_clause)


Suggested change

.map(Unnormalized::skip_norm_wip)

.filter_map(ty::Clause::as_trait_clause)

.map(Unnormalized::as_trait_clause)

View changes since the review

lcnr · 2026-04-15T15:37:03Z

                    queue.extend(
                        tcx.explicit_super_predicates_of(trait_pred.def_id())
                            .iter_identity_copied()
+                            .map(Unnormalized::skip_norm_wip)


explicit_super_predicates_of still does Unnormalized<(Clause, Span)>?

View changes since the review

lcnr · 2026-04-15T15:53:20Z

+                        let trait_ref = trait_ref.skip_norm_wip();
                        (
-                            trait_ref.to_host_effect_clause(
+                            Unnormalized::new_wip(trait_ref.to_host_effect_clause(


also add an fn to_host_effect_clause to Unnormalized

View changes since the review

lcnr · 2026-04-15T16:11:10Z

            // Don't check non-defaulted params, dependent defaults (including lifetimes)
            // or preds with multiple params.
-            if instantiated_pred.has_non_region_param()
+            if instantiated_pred.probe(|v| v.has_non_region_param())


why probe over skip_norm_wip?

View changes since the review

lcnr · 2026-04-15T16:11:37Z

                        .try_normalize_erasing_regions(
                            ty::TypingEnv::non_body_analysis(tcx, def_a.did()),
-                            unnormalized_ty,
+                            unnormalized_ty.clone(),


why .clone? Unnormalized should also be copy 🤔

View changes since the review

lcnr · 2026-04-15T16:12:55Z

                .tcx
                .explicit_super_predicates_of(def_id)
                .iter_identity_copied()
+                .map(Unnormalized::skip_norm_wip)


Suggested change

.map(Unnormalized::skip_norm_wip)

View changes since the review

lcnr · 2026-04-15T16:13:46Z


-    if let Err(reported) = tcx.type_of(start_from_impl).instantiate_identity().error_reported() {
+    if let Err(reported) =
+        tcx.type_of(start_from_impl).instantiate_identity().skip_norm_wip().error_reported()


Suggested change

tcx.type_of(start_from_impl).instantiate_identity().skip_norm_wip().error_reported()

tcx.type_of(start_from_impl).instantiate_identity().skip_normalization().error_reported()

intended, don't care about norm

View changes since the review

lcnr · 2026-04-15T16:20:47Z

+    pub fn probe<F, U>(&self, f: F) -> U
+    where
+        F: FnOnce(&T) -> U,
+    {
+        f(&self.value)
+    }


I would entirely remove this method/is it needed to get something that's copy out of an Unnormalized which is not? 🤔 maybe add as_ref() instead then, as in value.as_ref().skip_normalized().whatever

View changes since the review

lcnr

Things I'd like to see in this PR maybe?

TypeErrCtxt::normalize_fn_sig should take Unnormalized. that should simplify the diff
explicit_item_self_bounds is an iterator over Unnormalized<(Whatever, Span)>, it should also have the span outside of it

Things for future PRs

I dislike .map(|(c, s)| (c.skip_norm_wip(), s)) as a pattern, we should skip_norm_wip once we iterate over the collection
all of the t == cx.tcx.normalize_erasing_regions(cx.typing_env(), t) asserts should be some assert_fully_normalized and don't use Unnormalized::new_wip
a bunch of these normalize calls in codegen are unnecessary. we should change them to use assert_fully_normalized instead
a big one is struct_tail_raw whose normalize should take an Unnormalized
all calls to skip_norm_wip().def_id should be replaced with a def_id() function on Unnormalized<Whateverhasadefid>
we should add fn Unnormalized::<T>::map<U>(self, f: impl FnOnce(T) -> U) -> Unnormalized<U>
unsure what to do in type_of and predicates_of as these queries very intentionally ferry unnormalized types around. I guess we just have a .skip_normalization() for places where it's intentional 🤔
also slightly unsure about elaborate, I think it should take Unnormalized as it also just returns unnormalized things?
field.ty should return Unnormalized
normalize_with_depth_to should take Unnormalized 🤔
coherence::ImplHeader should its things be Unnormalized I think?
DropckConstraint should have Unnormalized fields? 🤔
the DeeplyNormalize QueryTypeOp should contain an Unnormalized<T> as input

View changes since this review

lcnr · 2026-04-15T16:21:40Z

reviewed all changes, and would like to merge this soon 😁 makes it easier to work off it