Description
Right now, we have two "const_eval" queries: const_eval_raw is internal and returns a "by-reference" constant (MPlaceTy) pointing to some global allocation in tcx.alloc_map. And const_eval is the entry point supposed to be used by the rest of the compiler, and it returns a ty::Const, representing some constants of some types "by-value".
Beyond the by-ref to by-val conversion, const_eval also performs validation of the constant to make sure it matches its type. The fact that const_eval_raw does not do that is a constant source of concern for me, e.g. we have some delay_span_bug in that query that basically rely on validation happening eventually and I think some of those might actually be "exploitable" ICEs.
Furthermore, "validated or not" as a question seems separate from "by-val or by-ref". By-val representation is appropriate for many uses of const (in particular around array lengths and pattern matching), but AFAIK never makes any sense for static. We currently force statics to go through ty::Const which seems odd. In fact, in the context of rust-lang/const-eval#42 and #70889, we determined that pattern matching and const generics likely want a different, even more higher-level representation of const values than what we currently have for ty::Const.
So I propose we perform validation in const_eval_raw (whose name should be changed then... maybe const_eval_ref?). Then all these delay_span_bug issues disappear. But the query should still return an MPlaceTy. The idea is that this is the query used for static and other clients (like Miri) that can handle constants of any type and only care about their value at the byte level.
Then we can have a second query which, like current const_eval (new name maybe const_eval_val?), returns ty::Const. This would only be used for things like array lengths, pattern matching, const generics -- things which need a more "structured" view of consts than MPlaceTy provides. I think this would be a great place to put some check that "all allocations reachable from a const are read-only" (as proposed here). Also, my hypothesis is that this query should only ever be called on "structural match" types -- maybe we can assert that? Long-term, this could become the "integer tree" representation @eddyb proposed.
The main technical hurdle is that we have to avoid query cycles. That's why validation was split out into a separate pass in the first place. We can achieve that by stopping validation when we hit a GlobalAlloc::Static(DefId) allocation. I think that is reasonable -- we already stop there if the DefId is in a different crate, so we cannot rely on this for soundness anyway.
@rust-lang/wg-const-eval does this all make sense?
I think if this works it quite be a really nice cleanup, and I would finally be much less bothered by the existence of two queries and two different representations of consts. :D Some time ago I thought we could kill ty::Constand use by-ref consts for everything, and I think @oli-obk even tried that and it went nowhere. But "by-ref" just seemed like the canonical representations of consts, and we ended up with ugly hack where whether we are in a static affects how we convert MPlaceTy into ty::Const... ugh. Now I finally understand what that is the case: we have two different kinds of consumers of consts in the compiler, with different needs, and they should use different queries with different return types. Some are happy with MPlaceTy, others require a more high-level view. I feel like we finally found the right structure to represent this. :D