argument-less const fn do not get properly executed

[RalfJung]

With rust-lang/rust#66294 landed, argument-less const fn get memoized even when run in Miri. I am pretty sure that is wrong as they can (certainly through unsafe code, once that is allowed) have arbitrary side-effects.

Cc @davidhewitt @oli-obk

[oli-obk]

cc @Centril wrt side effects in const fn.

My stance is that it's UB to do side effects in const fn

[RalfJung]

Well right now, with "Miri unleashed", this bug means we miss UB. So whatever your stance is, we have to change something. If you don't like my proposal of restricting memoization to CTFE, please make another one.

[Centril]

My stance is that it's UB to do side effects in const fn

I agree with that, at least until we say otherwise. :)

[RalfJung]

My stance is that it's UB to do side effects in const fn

Given that Miri is all about detecting UB I don't see how that argument helps here at all?

The discussion of how to expand UB to incorporate const fn properties is a separate one, and IMO a serious proposal of that requires a demonstration that we can check this UB in Miri. But in no way can that be an argument for removing UB checks from Miri now before any decisions we even made.

[RalfJung]

My interpretation is that that PR landed because nobody realized that this would affect Miri as well, and we should revert its Miri effect ASAP before doing anything else. Then we can talk about expanding our UB and making Miri check it or whatever.

Is that not correct? If Miri behavior was deliberately changed by that PR I am surprised I did not get Cc'd. I'd prefer to be in the loop for such things.

[oli-obk]

If Miri behavior was deliberately changed by that PR I am surprised I did not get Cc'd. I'd prefer to be in the loop for such things.

Sorry, I keep forgetting this. Not sure how to fix that except remind myself regularly of it. I'll strive to do better.

My interpretation is that that PR landed because nobody realized that this would affect Miri as well, and we should revert its Miri effect ASAP before doing anything else.

I now see that I did not realize

Given that Miri is all about detecting UB I don't see how that argument helps here at all?

The change is actually a language change now, because it only looks at the result of a const eval and not at the intermediate computation. For const eval that's not a problem, so I'll make a PR allowing miri to still run the function

[RalfJung]

The change is actually a language change now, because it only looks at the result of a const eval and not at the intermediate computation.

A language change in Miri, you mean? Yes, that's why I raised this issue.
We don't also use the memoized result during codegen, do we? Certainly with function pointers that seems hard.

so I'll make a PR allowing miri to still run the function

Thanks!

[oli-obk]

We don't also use the memoized result during codegen, do we? Certainly with function pointers that seems hard.

No, but in the future MIR optimizations may optimize out such function calls and just put their result into the MIR

[RalfJung]

in the future MIR optimizations may optimize out such function calls and just put their result into the MIR

That is a language change requiring some new UB though (at least when considering Rust with Miri unleashed) -- so I hope we don't do this before we can detect such UB.

[RalfJung]

Fixed by rust-lang/rust#66866