unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
rust-lang/rust
senior8/10
type-systemscompiler-internalserror-handling

Make const eval return normalized ty

Project: rust-lang/rust | Language: Rust | Concepts: type-systems, compiler-internals, error-handling | Difficulty: senior
Link: https://github.com/rust-lang/rust/pull/160526

The problem

When evaluating constants in the Rust compiler's trait solver, the type of the resulting value was not being normalized. This could lead to situations where the type of a constant (e.g., <Type as Trait>::Matrix) was not reduced to its actual concrete type (e.g., [usize; 1]). This lack of normalization could cause internal compiler errors (ICEs) or incorrect behavior downstream, such as during symbol name mangling or type checking.

Why it's non-trivial

A naive fix might simply attempt to normalize types after constant evaluation everywhere constants are used. However, normalization must happen at the right place and time, and must propagate errors correctly. Additionally, the trait solver's evaluation machinery is layered and interacts with both infallible and fallible normalization routines. Handling all possible error cases (e.g., type normalization failures, generics still present, or CTFE failures) without conflating them is subtle and requires careful API changes.

The approach

The PR makes the following key changes:

1. Change the trait for evaluating consts to require normalization

-    fn evaluate_const(
+    /// Evaluate a const, normalizing the type of the resulting value with `normalize_ty`.
+    /// Returns `Ok(None)` if the const is too generic, and `Err(_)` only if `normalize_ty`
+    /// failed.
+    fn evaluate_const<E: Debug>(
         &self,
         param_env: <Self::Interner as Interner>::ParamEnv,
         alias_const: ty::AliasConst<Self::Interner>,
-    ) -> Option<<Self::Interner as Interner>::Const>;
+        normalize_ty: impl FnOnce(
+            ty::Unnormalized<Self::Interner, <Self::Interner as Interner>::Ty>,
+        ) -> Result<<Self::Interner as Interner>::Ty, E>,
+    ) -> Result<Option<<Self::Interner as Interner>::Const>, E>;

Explanation:
The evaluate_const method now requires a normalization function to be provided, and returns a Result that can propagate normalization errors. This ensures that the type of the evaluated constant is always normalized, and that normalization failures are handled as errors, not as ambiguities.

2. Thread normalization through all callers

All calls to evaluate_const and try_evaluate_const are updated to pass a normalization function. For example:

-        Ok(self.delegate.evaluate_const(param_env, alias_const))
+        self.delegate.evaluate_const(param_env, alias_const, |ty| {
+            self.normalize(GoalSource::Misc, param_env, ty)
+        })

Explanation:
The normalization function is now explicitly provided, using the solver's own normalization capability. Errors from normalization are now propagated as part of the result.

3. Update error handling and result types

The error enum is made generic to carry normalization errors:

-#[derive(Debug)]
-pub enum EvaluateConstErr {
+pub enum EvaluateConstErr<E> {
    ...
+    FailedNormalization(E),
}

Explanation:
This allows normalization errors to be propagated through the same error channel as other constant evaluation errors.

4. Normalize the type after CTFE

-                    Ok(ty::Const::new_value(tcx, val, alias_const.type_of(tcx).skip_norm_wip()))
+                    let ty = normalize_ty(alias_const.type_of(tcx))
+                        .map_err(EvaluateConstErr::FailedNormalization)?;
+                    Ok(ty::Const::new_value(tcx, val, ty))

Explanation:
After evaluating the value of a constant, its type is normalized using the provided function, and any failure here is propagated as a new error variant.

5. Update all call sites to handle new error/result shapes

For example, in trait selection and const-evaluatable checks:

-                match crate::traits::try_evaluate_const(infcx, unexpanded_ct, param_env) {
+                match crate::traits::try_evaluate_const(infcx, unexpanded_ct, param_env, |ty| {
+                    Ok::<_, !>(ty.skip_norm_wip())
+                }) {

Explanation:
All invocations now pass a normalization function, even if it's an infallible one (e.g., skip_norm_wip()), and handle the new error types.

6. Add regression and behavior tests

New tests verify that the type of evaluated constants is normalized, and that code using such constants compiles or errors as expected.

+// Regression test for https://github.com/rust-lang/rust/issues/156294.
+// We used to not normalize the type we get back from const evaluation, so the value of
+// `EMPTY_MATRIX` had the type `<Type as Trait>::Matrix` instead of `[usize; 1]`. ...

From the review thread

khyperia: neat, looks good to me!

one mostly style comment I had is that the evaluate machinery is getting really annoyingly tangly and layered and messy, and this is just more layers of wonkiness on top, haha. I wonder if instead of having a double-wrapped Result like you do with this change, instead:

  • remove the _with_fallible_normalization methods
  • EvaluateConstErr gets a new variant, "type normalization error", whose value is a generic parameter
  • try_evaluate_const has an <E> generic arg, takes a normalize: fn(..) -> Result<Ty, E> and returns Result<Const, EvaluateConstErr<E>>
  • evaluate_const returns either Result<Option<Const>, E> or Result<Const, SomeNewEnum<E>> that's either... you know what, hm, evaluate_const maybe should just get nuked, maybe everything should use try_evaluate_const, since there's multiple complex failure modes of regular evaluate_const, and the complexity reduction of evaluate_const over try_evaluate_const isn't that useful anymore. idk. shrug.
  • infallable normalization returns Result<Ty, Infallible> so try_evaluate_const returns EvaluateConstErr<Infallible>. (or actual ! never, I forget if we're allowed to use it here)
  • fallable normalization returns with E=NoSolutionOrRerunNonErased or whatever
  • try_evaluate_const_inner doesn't need to exist, nor does the weird new enum EvaluatedConst

anyway, just kind of spitballing here, totally up to you on whether you try to clean this up, it's also fine as-is (I'll probably try to clean it up post-merge though if you leave it as-is)

khyperia: Yeah, the next morning's coffee in hand with a proper sleep, I think I agree, on a theoretical level a type normalization failure here should be an error, not treated as ambiguous (or, under full GCA, as rigid). I think the None case ought to only come from CTFE needing concrete types to compute values, but types ought to always be "evaluable" (even if just to some rigid version of themselves if generics are involved).

On a concrete technical level, both callers already return Result<_, NoSolutionOrRerunNonErased> (well, spelled QueryResultOrRerunNonErased<I> but whatever, same thing) and both already do ?, so on a mechanical level it's straightforward. I think the match statement here would also just go away, return self.delegate.evaluate_const directly~

Unfortunately I'm also unsure of whether it's even possible to reach this point. I spent a bit trying to manually fuzz my way into hitting it and was unable to. I'll poke in zulip to see if people who know more than I do know what's up :3

Takeaways

  • Always normalize types after evaluating constants in a type system. Failing to do so can lead to subtle bugs or ICEs, as the type may not be in its most reduced (concrete) form.
  • Propagate all possible errors explicitly through result types. Do not conflate different error cases (e.g., normalization failure vs. "too generic"); use enums and generics to make error handling precise.
  • When changing core APIs, update all callers and add regression tests. This ensures that the new behavior is exercised and that subtle issues (like type normalization) do not regress in the future.