Validate constants during const_eval_raw

[oli-obk]

This PR implements the groundwork for #72396

• constants are now validated during const_eval_raw
• to prevent cycle errors, we do not validate references to statics anymore beyond the fact that they are not dangling
• the const_eval query ICEs if used on static items
• as a side effect promoteds are now evaluated to ConstValue::Scalar again (since they are just a reference to the actual promoted allocation in most cases).

[rust-highfive]

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

[petrochenkov]

r? @RalfJung

[RalfJung]

I'm going to also change it in this PR so that the const_eval query may not be used for statics anymore

Are you also going to do the renames we talked about? Where the "raw query" returns a mir::Const, and then maybe that query is called const_eval and the one returning ty::Const is called const_eval_for_ty or so.

One problem I realized for this is that consumers that want to work with mir::Const still have to also be able to handle ty::Const embedded in MIR as operands -- it's just that if they encounter an Unevaluated they want to use mir::Const. Actually, does it really make sense for Operand to use ty::Const? This is not in a type after all, and all sorts of consts are supported! So mir::Const seems more appropriate. Except maybe for efficiency reasons we want to sometimes not generate a tcx Allocation backing this const (does this ever happen?), so the type we want would be more like "mir::Const or ty::Const". We probably also need that because MIR can refer to const generics.

[oli-obk]

Are you also going to do the renames we talked about?

Idk, should I do them in the same PR? These are pretty invasive to a lot of code, so mixing them with this PR may make it hard to distinguish functional from non-functional changes.

Actually, does it really make sense for Operand to use ty::Const?

no, this would be exactly the site where we'd use mir::Const (or a multi-type as you suggested). What other sites did you have in mind for mir::Const?

Except maybe for efficiency reasons we want to sometimes not generate a tcx Allocation backing this const (does this ever happen?), so the type we want would be more like "mir::Const or ty::Const". We probably also need that because MIR can refer to const generics.

yes, I was not going to do this change in this PR, as it has its own complexity problems and I worry about the reviewability of such a huge change (I have a local branch working on this seperately).

[RalfJung]

Looks great overall. :) But I have a few comments and nits.

[RalfJung]

Actually, does it really make sense for Operand to use ty::Const?

no, this would be exactly the site where we'd use mir::Const (or a multi-type as you suggested). What other sites did you have in mind for mir::Const?

Mostly the return type of the const_eval_static query.

[bors]

☔ The latest upstream changes (presumably #75383) made this pull request unmergeable. Please resolve the merge conflicts.

[RalfJung]

The Miri-affecting change landed so I think this is unblocked.

Btw I think the PR description needs updating.

[bors]

☔ The latest upstream changes (presumably #75797) made this pull request unmergeable. Please resolve the merge conflicts.

[oli-obk]

@bors r=RalfJung

[bors]

📌 Commit 34785fc has been approved by RalfJung

[bors]

⌛ Testing commit 34785fc with merge 5e449b9...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: RalfJung
Pushing 5e449b9 to master...

[rust-highfive]

📣 Toolstate changed by #74949!

Tested on commit 5e449b9.
Direct link to PR: #74949

💔 miri on windows: test-pass → build-fail (cc @oli-obk @eddyb @RalfJung).
💔 miri on linux: test-pass → build-fail (cc @oli-obk @eddyb @RalfJung).

[matthiaskrgr]

Looks like this introduced some perf regressions https://perf.rust-lang.org/compare.html?start=10b3595ba6a4c658c9dea105488fc562c815e434&end=5e449b9adff463455743291b0c1f76feec092992&stat=instructions:u

[RalfJung]

Uff, 500% on the stress test is heavy.^^ I guess we'll have to revert? :(

[Mark-Simulacrum]

Note that it's just incremental getting worse on ~everything that I checked, so I suspect that probably we're invalidating caches more somewhere -- maybe fixable?

But FWIW I am not too worried about incremental perf when the trade-off is worse code or poor checking. Though of course if we can avoid regressing it that would be good :)

[oli-obk]

I'll investigate.

[oli-obk]

lol found it: eval_to_allocation_raw is not cached on disk at all.

[oli-obk]

Fix PR: #77006

[ecstatic-morse]

Hi! This PR showed up in the weekly perf triage report. It resulted in a very large regression in instruction counts of incremental builds (up to 515.8% on incr-unchanged builds of ctfe-stress-4-check).

Looks like this is already fixed. Nice work all!