Intrinsics should allow a non-const fallback body if hooked for CTFE

[tgross35]

Currently if an intrinsic is marked const, its body needs to be const as well. This makes sense when the body is providing the comptime implementation, but is limiting in cases where CTFE hooks the function so the fallback would only ever be called in non-const contexts.

Noticed in #150946

[RalfJung]

Cc @rust-lang/wg-const-eval

I think the hardest part here is to decide about the syntax to distinguish "const intrinsic with const fallback body" and "const intrinsic with non-const fallback body".

[tgross35]

I think the hardest part here is to decide about the syntax to distinguish "const intrinsic with const fallback body" and "const intrinsic with non-const fallback body".

Perhaps they should use the same syntax but a #[const_intrinsic_must_hook] attribute opts the second case out of a const body? I was going to suggest removing the const keyword in the for "const intrinsic with non-const fallback body", but then decided that making the signature appear correct to callers and in docs (given it can be called in const) is more important than matching up with whatever is going on in the body, since that's already pretty magic.

[RalfJung]

Yeah that sounds plausible.

[RalfJung]

It seems like we actually already have an ad-hoc way of doing this, and we're using it for at least one intrinsic:

#[rustc_intrinsic]
#[rustc_nounwind]
#[rustc_do_not_const_check]
#[inline]
#[miri::intrinsic_fallback_is_spec]
pub const fn ptr_guaranteed_cmp<T>(ptr: *const T, other: *const T) -> u8 {
    (ptr == other) as u8
}

The rustc_do_not_const_check means we're not const-checking the body, and const-eval should error if we ever accidentally use that body and don't overwrite it.

[RalfJung]

Cc @folkertdev @N1ark since you were also involved in some of the discussions around intrinsic fallbacks.

[N1ark]

#[rustc_do_not_const_check] seems like a Fine solution no? or would you want something more specific?

[RalfJung]

I guess it is an okay lego brick solution, though IMO it should always have a comment. Maybe we should require rustc_do_not_const_check(reason = "interpreter has built-in implementation")? It's a bit fragile since nothing ensures that indeed the interpreter has an implementation. So every such intrinsic must have a smoke test invoking it in const context.

[RalfJung]

It's a bit fragile since nothing ensures that indeed the interpreter has an implementation.

Though to be fair that's the same with const intrinsics without a fallback body. Nothing checks your const annotations.