rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.65k stars 12.75k forks source link

Tracking issue for removing unnecessary `ty::Const::{normalize,eval}` calls from the type system #130704

Open compiler-errors opened 1 month ago

compiler-errors commented 1 month ago

We want to remove calls to ty::Const::{normalize,eval,eval_to_*} because they won't work correctly with future reformulations of GCE, and because they're unnecessary with non-GCE consts. This issue tracks doing that so I won't forget. Boxy can probably write more motivation here idk

Boxy rationale:

with min_generic_const_args and associated_const_equality featuers normalization of type system constants will be behaving much more like types. They'll return nested goals, access in scope whjere clauses such as T: Trait<ASSOC = 10> in order to normalize instead of simply "evaluating".

This means that the only correct way to normalize a ty::Const will be to use the "normal" normalization routines such as normalize_erasing_regions or infcx/ocx/fcx.normalize. With that in mind all of the eval_x and normalize methods on ty::Const become massive footguns as they are never correct to use.

More:

cc @BoxyUwU

RalfJung commented 1 month ago

and because they're unnecessary with non-GCE consts

Are they? The role of ty::Const::normalize is to bring this constant into the shape ConstKind::Value where possible (so that e.g. it can be turned into a concrete integer without any further interpreter invocations). This is entirely orthogonal to GCE.

RalfJung commented 1 month ago

As a meta remark here, I think @rust-lang/wg-const-eval should generally be kept in the loop when t-types is fundamentally changing the API of ty::Const, as that API is clearly in the responsibility of both teams/groups. :)

In particular, we did a bunch of work to make that API somewhat consistent with mir::Const, for good reasons. (Type systems consts can be used as MIR values, so every operation on mir::Const has to be supported by ty::Const.)

BoxyUwU commented 1 month ago

Nothing is fundamentally changing about being able to normalize type system constants, this has zero impact on const evaluation. This is a refactoring to make ty::Const's API be less error prone when const aliases are involved, not to remove functionality.

RalfJung commented 1 month ago

I didn't say any functionality was being removed. Just APIs are being (re)moved, for what I assume are good reasons. But surely wg-const-eval needs to understand why this is done. You can't say that the ty::Const API is not relevant for wg-const-eval...

BoxyUwU commented 1 month ago

I am absolutely saying that in this case it has nothing to do with this group. Nothing has fundamentally changed with this refactoring, it has no impact on const eval whatsoever. This is primarily a focus about how the API should be structured for normalizing type system constants not any of the actual const eval logic.

If we were introducing new constraints on how ctfe must work for const generics or what circumstances ctfe must be able to run in then sure wg-const-eval should be included. But this refactoring does not affect const eval/ctfe in any real way.

fee1-dead commented 1 month ago

Both of you may be interested in moving extended discussion into a Zulip topic :)