Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relax const-eval restrictions #3352

Closed

Conversation

Noratrieb
Copy link
Member

@oli-obk oli-obk added the A-const-eval Proposals relating to compile time evaluation (CTFE). label Dec 2, 2022

If a `const fn` is called in a runtime context, no additional restrictions are applied to it, and it may do anything a non-`const fn` can (for example, calling into FFI).

A `const fn` being called in a const context will still be required to be deterministic, as this is required for type system soundness. This invariant is required by the compiler and cannot be broken, even with unsafe code.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that, for example, CTFE can't use host floats for anything that might be NAN, because LLVM might change the NANs when run at different times, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

@scottmcm scottmcm added the T-lang Relevant to the language team, which will review and decide on the RFC. label Dec 3, 2022
# Rationale and alternatives
[rationale-and-alternatives]: #rationale-and-alternatives

An alternative is to keep the current rules. This is bad because the current rules are highly restrictive and don't have a significant benefit to them. With the current rules, floats cannot be used in `const fn` without significant restrictions.
Copy link
Member

@scottmcm scottmcm Dec 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to see a sketch of what "significant restrictions" might actually look like. It's hard for me to weigh "not significant" vs "significant" without more details.

For example, some of the provenance discussion basically ended up at "we at least need ______ because LLVM & GCC both require that, and it's a non-starter to require a completely new optimizing codegen backend". I'd love to tease out more how bad extra things would be. "Painful but we've done things like it before", like symbolic pointer tracking, is a very different consequence from, say, "it's an open research question if it's even possible".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rust-lang/rust#77745 has a lot more info on that, I will look through it again to sketch out some concrete ideas for const floats under the current rules once I have time (unless someone else wants to do that :)).

But from what I recall:
There are really two ways to restrict this. We could try to ensure this dynamically by erroring out as soon as problematic values (like NaNs or subnormals) are produced. This should be compatible with the current rules, because even though there are differences between const and runtime, but these rules are unobservable from code. This would be a restriction, but should not be a big one for most code.
Alternatively, we could statically forbid all operations that could lead to NaNs or subnormals in const fn. This is not realistic.
There's also the option of using softfloats at runtime inside const fn but that's not a realistic option either.

So actually I do think that floats inside const fn are possible under the current rules without too mnay restrictions. But this RFC still makes it simpler and obvious. (and also comes with many other benefits aside from floats).


Also, floats are currently not supported in `const fn`. This is because many different hardware implementations exhibit subtly different floating point behaviors and trying to emulate all of them correctly at compile time is close to impossible. Allowing const-eval and runtime behavior to differ will enable unrestricted floats in a const context in the future.

Rust code often contains debug assertions or preconditions that must be upheld but are too expensive to check in release mode. It is desirable to also check these preconditions during constant evaluation (for example with a `debug_or_const_assert!` macro). This is unsound under the old rules, as this would be different behavior during const evaluation in release mode. This RFC allows such debug assertions to also run during constant evaluation (but does not propose this itself).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hint at another possible alternative, to me: treating diverging differently from the results of the evaluation.

After all, it's normal that two things with the same postcondition can actually have different behaviour, if they need to diverge to communicate that they cannot uphold the promised post-condition.

(That certainly doesn't solve nondeterministic NANs, but might address a useful subset.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like diverging is always a valid possibility even under the current behaviour, since we already do this with things like overflow: const evaluation will always fail on overflow, whereas overflow might wrap at runtime. Or like, the power could go out suddenly and stop the program during runtime, but by nature of the program already being compiled, clearly it successfully computed everything during compile time.

I mean, I know that people are clearly discussing this already in cases like that, but I feel like this kind of restriction is something that could be removed even without this RFC, whereas this RFC proposes things like, f32::sin could compute the sine in degrees in constant evaluation and radians at runtime. (Obviously that would be a horrible idea, but nonetheless possible.)

@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 3, 2022

From what I remember, C++ library used the rule "possible constexpr behaviors of X must be a subset of possible runtime behaviors of X" when adding constexprs to functions.

This rule covers a lot of cases starting from imprecise results with floating points, and ending with asserts as a subset of undefined behavior for functions with broken pre-requisites.

const fn-based optimizations should also be possible in this case.

@clarfonthey
Copy link
Contributor

This feels a lot to me like the entire mem::forget discussion all over again, except in this case, there's no existing safe method to do this; the only way is via const_eval_select, currently.

Drawing the connection to that discussion, I definitely think that this is something we want to do, but I also feel like there needs to be more emphasis put on safety here. I don't have any concrete ideas of how this could happen, but with mem::forget at least, there's the added caveat that, because calling mem::forget is safe, code can't rely on things not being leaked for safety. This is relevant in things like draining iterators, which must ensure that forgetting the iterator doesn't allow the caller to observe dropped values.

To put it more clearly: I think that this RFC should be explicit about the fact that, while the behaviour may differ between const and runtime contexts, programs should not be allowed to rely on which context is being run for safety. This, in effect, would nullify this clause:

Pure const fn under the old rules can be seen as a simple optimization opportunity for naive optimizers, as they could just reuse constant evaluation for const fn if the argument is known at compile time, even if the function is in a runtime context. This RFC makes such an optimization impossible.

In other words, I feel like this RFC shouldn't explicitly rule out the possibility of dynamically evaluating constant values at runtime (via inlining or otherwise) or statically evaluating intermediate values at compile time (for example, simplifying mathematical expressions). The compiler won't automatically do this, of course, but it could be useful for testing purposes or so-called "risky" optimizations; these are normally called unsafe, but we explicitly prevent them from being unsafe with this clause.

Note: I may just be overcomplicating this and this clause would be a bad idea. But I figure it's worth talking about in case someone has a real example of this.

@ChayimFriedman2
Copy link

What was the reason for the limitation to begin with?

@oli-obk
Copy link
Contributor

oli-obk commented Dec 4, 2022

What was the reason for the limitation to begin with?

It required additional logic to be able to differ, so the original implementation didn't. This RFC is the result of effectively years of discussions on this topic. We hope that anyone with a good use case for keeping the status quo will speak up, because so far we've only been able to create artificial examples

@comex
Copy link

comex commented Dec 4, 2022

No strong opinions about this, but I'd like to point out that 'different behavior at runtime is UB' and 'different behavior at runtime is fine' are not the only possible options. Another option is to say 'calls to the function nondeterministically produce either the runtime or the const-eval behavior'. In other words, the compiler is allowed to take calls to const fns with known arguments and replace them with const-evaluated results, but it can't assume that doing so keeps the program's behavior the same.

When it comes to real examples where such an optimization might be useful: look at uses of the GCC extension __builtin_constant_p, particularly in combination with inline assembly. For example, Linux has the following definition for a macro equivalent to u8::count_ones:

#define hweight8(w)  (__builtin_constant_p(w) ? __const_hweight8(w)  : __arch_hweight8(w))

That is, if w is known to be constant, calculate the answer using __const_hweight8, otherwise calculate it using __arch_hweight8.

__const_hweight8 is a pure-C implementation, which would (probably) be slow at runtime but is fine if the compiler is constant-folding it:

#define __const_hweight8(w)		\
	((unsigned int)			\
	 ((!!((w) & (1ULL << 0))) +	\
	  (!!((w) & (1ULL << 1))) +	\
	  (!!((w) & (1ULL << 2))) +	\
	  (!!((w) & (1ULL << 3))) +	\
	  (!!((w) & (1ULL << 4))) +	\
	  (!!((w) & (1ULL << 5))) +	\
	  (!!((w) & (1ULL << 6))) +	\
	  (!!((w) & (1ULL << 7)))))

__arch_hweight8, on the other hand, typically expands to an inline assembly block using the CPU's popcount instruction. This is fast at runtime, but (like all inline assembly) completely opaque to the compiler and unable to be constant-folded.

That said, one reason this works well in C is that __builtin_constant_p is preserved all the way through the optimization pipeline. It returns true not only if the argument is a language-level constant expression, but also if it's been optimized into a constant. This is quite different from Rust const evaluation (or C++ constexpr evaluation, for that matter) which, at least for the foreseeable future, can only be evaluated earlier in the pipeline than most optimizations.

Indeed, an alternative way to support the above use case would be to just expose __builtin_constant_p itself - or rather @llvm.is.constant.i32, the LLVM intrinsic that Clang lowers __builtin_constant_p to - as a separate API.

@RalfJung
Copy link
Member

RalfJung commented Dec 4, 2022

No strong opinions about this, but I'd like to point out that 'different behavior at runtime is UB' and 'different behavior at runtime is fine' are not the only possible options. Another option is to say 'calls to the function nondeterministically produce either the runtime or the const-eval behavior'.

Yet another option is to say that the language semantics and compiler don't make any assumptions, but the safety invariant of such functions guarantees that replacing the runtime version by the compile-time version is fine. (That's basically @petrochenkov's subset property, but as a safety invariant instead of a validity invariant.)

All of these have in common that if we ever guarantee that certain hardware-specific aspects e.g. of floating point NaN signs are observable in Rust, then we'd have to also guarantee that CTFE behaves in that way.

From what I remember, C++ library used the rule "possible constexpr behaviors of X must be a subset of possible runtime behaviors of X" when adding constexprs to functions.

FWIW this is insufficiently defined -- if the function has type const fn myfun(f: const fn(i32) -> i32), then to even talk about "set of behaviors" you need to define which set of functions f you are considering. All safe functions? All sound functions? All syntactically valid functions (including arbitrary use of unsafe)?

This is also closely related to @comex' proposal -- by saying that we pick const or runtime code non-deterministically at runtime, we make it trivially the case that const behavior is a subset of runtime behavior.

So what would that mean for functions like https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.guaranteed_eq? I guess we'd have to say that technically the runtime version is always allowed to return None, even though in practice it never will? Not sure if that kind of "specification trick" really helps anyone. We'd basically specify that the runtime behavior is a big overapproximation of the actual implementation -- that is technically correct, but if we learn anything from align_to it's that this leads to endless frustration. All of these proposals would make it impossible to have a const fn align_to documented as "at runtime, align_to will definitely return a maximally sized middle part; only at compile-time can it be the case that all elements unnecessarily end up in the head or tail".

@clarfonthey
Copy link
Contributor

No strong opinions about this, but I'd like to point out that 'different behavior at runtime is UB' and 'different behavior at runtime is fine' are not the only possible options. Another option is to say 'calls to the function nondeterministically produce either the runtime or the const-eval behavior'. In other words, the compiler is allowed to take calls to const fns with known arguments and replace them with const-evaluated results, but it can't assume that doing so keeps the program's behavior the same.

I believe that this is a stronger version of my proposal -- I say simply that you should assume this in the case where UB may occur as a result, whereas you say this might occur whenever, even if the result would not trigger UB.

@comex
Copy link

comex commented Dec 4, 2022

All of these proposals would make it impossible to have a const fn align_to documented as "at runtime, align_to will definitely return a maximally sized middle part; only at compile-time can it be the case that all elements unnecessarily end up in the head or tail".

Interesting point.

Possible counterargument: If the semantic difference between "best-effort align_to" and "exact align_to" is important, then they really should be different functions with different names; making them the same function creates a footgun. After all, it's normally a safe refactoring practice to just blindly mark functions as const and, if they compile, assume that they do the same thing that they do at runtime. That wouldn't work if people wrote code that used align_to but depended on the stronger semantics. (This is especially footgunny because it's not immediately obvious why alignment can't be known at compile time.)

On the other hand, for eq and guaranteed_eq we do have two separate functions, yet I can imagine some code using, say, a.guaranteed_eq(b).unwrap(). The reasoning might be: 'At runtime we might be comparing anything, but at compile time we only compare pointers that the compiler knows are either the same or different. If we accidentally break that rule, no big deal, it'll just be a compile error due to a const-eval panic.' The author might then be surprised if the code started panicking at runtime. Even without const-eval at optimization time, such code would be non-portable, risking const-eval panics under different Rust compilers or rustc versions, since guaranteed_eq's documentation explicitly refuses to guarantee it returns Some in any circumstances (at least at compile time). But it might work in practice, at least once rustc's const-eval implementation gets a little smarter.

It's always hard to hold the door open for an optimization that's not actually being performed.

@RalfJung
Copy link
Member

RalfJung commented Dec 7, 2022

Possible counterargument: If the semantic difference between "best-effort align_to" and "exact align_to" is important, then they really should be different functions with different names; making them the same function creates a footgun.

I agree that some sort of opt-in to the "const-time weaker version" makes sense. See for example guaranteed_eq for what that could look like. However, even those functions we currently like to specify as

At runtime this function behaves like Some(self == other).

This is useful, for example, for the slice eq function which can have a fast-path. Sure, since this is just a fast-path, returning None even at runtime would technically be fine, but if the align_to experience shows anything it is that people are very paranoid about specification hedging of this sort.

Anyway the current guaranteed_eq docs already presuppose something like this RFC. If we don't want this RFC, we'll have to fix those docs to say that even at runtime this may return None.

In align_to I am currently trying to improve the situation without giving too strong guarantees via rust-lang/rust#105245. It seems like that is the minimum we have to guarantee to make people actually willing to use this function. It is somewhat unclear whether having guaranteed_eq with similar wording would be allowed under the policy of "compiletime code must only behave in ways that are also possible for runtime code".


Even without const-eval at optimization time, such code would be non-portable, risking const-eval panics under different Rust compilers or rustc versions, since guaranteed_eq's documentation explicitly refuses to guarantee it returns Some in any circumstances (at least at compile time).

For the sake of this argument, imagine we guaranteed it returned Some in some cases. (We totally could do that, we just haven't bothered.) So it is possible to write portable const-code that assumes that we sometimes get Some. That would make guaranteed_eq().unwrap() a bit more reasonable, no?

@dewert99
Copy link

dewert99 commented Dec 8, 2022

We hope that anyone with a good use case for keeping the status quo will speak up, because so far we've only been able to create artificial examples

I was able to come up with a couple of examples (not sure how good). They also rely on "definately const" bounds like in the keyword-generics proposal, and that return value const-fns doesn't depend on data behind interiour mutablity. Technically they don't inherently require the runtime results to match the compile time results (just that they are deterministic), but including these restrictions allows many of the added functions (eg InvWrap::new) to themselves be const-fns.
We could still allow const_assert!(exp) without breaking these examples as long as it expanded to something like const_assert_fn(|| exp) where const_assert_fn's argument type is impl Fn() -> bool to avoid issues with const_mut_ref.

#![feature(const_trait_impl)]
use crate::invariant::{Inv, InvWrap};

mod invariant {
    use std::marker::PhantomData;
    use std::ops::{Deref};
    use std::hint::unreachable_unchecked;
    
    #[const_trait]
    pub trait Inv<T> {
        fn apply(val: &T) -> bool;
    }


    pub struct InvWrap<T, I: const Inv<T>> (T, PhantomData<I>);

    impl<T, I: const Inv<T>> InvWrap<T, I> {
        pub fn new(val: T) -> InvWrap<T, I>{
            assert!(I::apply(&val));
            InvWrap(val, PhantomData)
        }

        pub fn with_mut(&mut self, f: impl FnOnce(&mut T)) {
            self.assume_inv();
            let inner = &mut self.0;
            f(inner);
            assert!(I::apply(inner))
        }
        
        fn assume_inv(&self) {
            if !I::apply(&self.0) {
                // SAFETY self could have only been created by new() which checks I::apply(&self.0)
                // self.0 is private so it can't be accessed directly
                // The only way to get a mutable reference to self.0 is through with_mut
                // which allow checks I::apply(&self.0) before the reference expires
                // and I::apply is a const fn so it will always return the same result
                unsafe { unreachable_unchecked() }
            }
        }
        
        pub fn into_inner(self) -> T {
            self.assume_inv();
            self.0
        }
    }


    impl<T, I: const Inv<T>> Deref for InvWrap<T, I> {
        type Target = T;

        fn deref(&self) -> &Self::Target {
            self.assume_inv();
            &self.0
        }
    }
}

mod trusted_ord {
    use core::cmp::Ordering;
    
    pub unsafe trait TrustedOrd {
        fn cmp(&self, other: &Self) -> Ordering;
    }
    
    #[const_trait]
    pub trait TrustedOrdRep {
        type Rep<'a>: TrustedOrd where Self: 'a;
        
        fn rep(&self) -> Self::Rep<'_>;
    }
    
    
    // Safety:
    // Since rep() is a const fn it will always return the same result,
    // so T::Rep::cmp will always be passed the same inputs.
    // Since it T::Rep implements TrustedOrd it's results are valid for a total order.
    unsafe impl<T: const TrustedOrdRep> TrustedOrd for T {
        fn cmp(&self, other: &Self) -> Ordering {
            self.rep().cmp(&other.rep())
        }
    }
}

struct IncreasingInvariant ();

impl const Inv<(u32, u32)> for IncreasingInvariant {
    fn apply(inner: &(u32, u32)) -> bool {
        inner.0 <= inner.1
    }
}

type IncreasingPair = InvWrap<(u32, u32), IncreasingInvariant>;

fn main() {
    let mut x = IncreasingPair::new((4, 8));
    x.with_mut(|p| {
        p.1 /= 4;
        p.0 /= 4;
    });
    assert_eq!(*x, (1, 2))
}

@RalfJung
Copy link
Member

RalfJung commented Dec 9, 2022

@dewert99 could you explain a bit what is happening there and where exactly the assumption comes in? Reverse engineering that from the code is quite a bit of work.

that return value const-fns doesn't depend on data behind interiour mutablity.

This is definitely not true. Cell::get will be a const fn eventually.

@dewert99
Copy link

dewert99 commented Dec 9, 2022

This is definitely not true. Cell::get will be a const fn eventually.

Hmm, if Cell::get can become a const fn, then the other const-eval restrictions seem less useful.

could you explain a bit what is happening there and where exactly the assumption comes in? Reverse engineering that from the code is quite a bit of work.

Sorry, the basic idea with InvWrap is to safely pass data with an invariant around in a way that helps the optimizer (eg. indexing a slice with an increasing pair could save a bounds check). Since we're using unsafe { unreachable_unchecked() } we need to ensure that I::apply(&self.0) must return true. We know it was true when created the InvWrap was created or after it was last mutated, but need I::apply to be deterministic (otherwise eg. we could make the invariant be that some file exists, then delete the file after creating the InvWrap and assume it still exists). Allowing InvWrap to implement Deref also requires that I::apply(&self.0) can't have its value changed by safe code given &self.0 which is why we needed the interior mutability requirement. Making InvWrap::new a const fn would also require that I::apply give the same result at compile-time as run-time (otherwise eg. it could return true at compile-time so InvWrap::new succeeds, but then false at run-time so we reach unreachable_unchecked().

@oli-obk
Copy link
Contributor

oli-obk commented Dec 9, 2022

This is not actually using the constness at all, afaict all calls of apply are runtime calls. So whatever guarantees you assumed either don't exist, or, assuming you meant to run the const-eval code at runtime (you are using const Trait, not ~const Trait), that's a funky feature that we could support, but I think that this would still work for you as you'd get the const-eval code path, which modulo floating point shenanigans would still be deterministic (or at least unaffected by this PR), even if run at runtime.

@dewert99
Copy link

dewert99 commented Dec 9, 2022

Sorry, @oli-obk you made me realize that the const Trait bounds probably aren't quite what I had in mind, using a system like https://blog.yoshuawuyts.com/const-syntax/ the bounds should probably be something like for<C> const<C> Trait meaning that the trait methods must callable at runtime and compiletime. The reason I didn't use ~const Trait was that runtime-only implementations of Trait would then be allowed as long as the methods in the impl block were runtime only.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 9, 2022

using a system like https://blog.yoshuawuyts.com/const-syntax/ the bounds should probably be something like for const Trait meaning that the trait methods must callable at runtime and compiletime. The reason I didn't use ~const Trait was that runtime-only implementations of Trait would then be allowed as long as the methods in the impl block were runtime only.

The ~const Trait syntax is just sugar for the hypothetical for<C> syntax. So if you don't mean that, then I'm unsure what feature you are describing.

Maybe open a zulip thread in the keywords generics stream so we can figure it out

@SUPERCILEX
Copy link

What are the remaining restrictions on const functions?

The RFC makes it sound like writing const fn would become obsolete because any function could be const. I'm assuming I've misread because that would be a huge deal and massively simplify the language. Tons of unstable features would be obsolete like const traits, for loops in const contexts, pointers, all the "make this fn const" features, etc.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 10, 2022

This RFC only removes the restriction that const fn must behave the same at runtime and at compile time. The difference between const fn and runtime fn is still absolutely meaningful.

@SUPERCILEX
Copy link

The difference between const fn and runtime fn is still absolutely meaningful.

Right, but what is that difference? The book calls out the fact that you can't write an rng in a const fn, but this RFC explicitly says that that would be possible, though it does say you'd only be able to do so in runtime contexts. Why is that distinction relevant though? If we let const fns do whatever, I think even build.rs would be obsolete since you could do whatever I/O you need as part of normal compilation.

I guess my question is why have any restrictions at all?

@Lokathor
Copy link
Contributor

A const fn called at const time must still return the same output for the same inputs. Even if it doesn't do that at runtime, it must still do that at const time.

@SUPERCILEX
Copy link

But why? What benefits does that have? Is there relevant discussion you could point me to? I can see determinism as a good argument, but you can get around that with build.rs. Also if someone wants nondeterminism in their builds, why not?

@Lokathor
Copy link
Contributor

one reason, though probably not the only one, is that const fn feeds into the type system (eg: array length), so if a const expression at const time does not have a single solution then your types get all messed up.

@SUPERCILEX
Copy link

if a const expression at const time does not have a single solution

Are you saying that the compiler reevaluates const fns every time it encounters them? If so, then yeah things would totally break, but it also doesn't seem that hard to cache the results within a compilation. I think this would suck for incremental compilation though since different compilation units could refer to the same const fn and be recompiled at different times. The semantics of when a const fn would be reevaluated during incremental compilation would also probably be really annoying to deal with.

All of that said, maybe solving those problems would actually result in a much simpler language. I don't think it's relevant for this RFC anymore, but something to chew on for the future.

@workingjubilee
Copy link
Member

workingjubilee commented Mar 7, 2023

A final(?) remark on the floating point thing: I agree with Muon, by and large. Though I think people absolutely are evaluating "interesting numerical code" in consteval already! This is in fact a standard sillybusiness in C++ with constexpr, especially for games programmers. Though they might want nondeterminism in floats too simply so that their const float evaluation can be more accurate instead of less in case they do, in fact, target thumbv7neon or whatever...

But I agree with Nilstrieb that this is "not about floats". Not only const_eval_select, but it also has to do with anything with implicit nondeterminism between const time and runtime that we want. For instance, we probably want a const-time heap. Without allowing nondeterminism in general, we have to carve out the specific exception that, yes, a programmer can allocate 100 GB on their 128GB RAM powerhouse workstation and use that as an intermediate input into a const evaluation, but then at runtime your program may have only 1GB available to it. So you're not allowed to have a single reasoning about system resources because the compilation and execution can literally happen on different systems... despite improved ability to reason about system resources being one of the benefits of Rust over many other languages.

Sure, exact system resources at runtime are inherently nondeterministic in many cases, but that really just makes it so much worse to be able to burn through a bonus set during consteval. We already have the complication that when LLVM elides a too-large allocation, that also can introduce a flavor of nondeterminism: a program that should have failed, doesn't, and that difference may be based on optimization level. The compile-time environment must implicitly "leak" into the program's runtime environment somehow in order to justify either of these things in a principled manner.

And then there's code that would like to be const fn but also use a HashMap with its RandomState at runtime and whew...

@RalfJung
Copy link
Member

RalfJung commented Mar 7, 2023

Resource exhaustion is a bit different, that's only non-deterministic about whether const-eval succeeds at all. A lot of heap-using code still has the property "if it gives an answer, it's always the same" (at compiletime and runtime).

@workingjubilee
Copy link
Member

That's true! That's why I left on the note of the HashMap and its randomness... such as its iteration order, despite being something you shouldn't poke at... suddenly becoming of significant impact and observability. Though I suppose const heap could simply exclude that, but as thin as libstd is, it seems kind of painful to have to exclude one of our premiere collections from code paths that could light up during both CTFE and runtime.

@joboet
Copy link

joboet commented Apr 15, 2023

I do understand the motivation and think it's probably a necessary step, but I really dislike this. With the current restriction, const indicates that the function is pure, and that allows cool things like writing more efficient sorting algorithms: By having a const Ord bound, cmp can never return different values for the same inputs, thereby allowing optimizations like unguarded insertion (where you compare against the firsts element first, and then use the result of comparing the to-be-inserted element to the other elements as the only loop condition to shift the element in the right place. Very efficient, but currently not possible in safe Rust). It seems a shame to need a separate pure specifier for this, when currently, const satisfies this need elegantly.

@CAD97
Copy link

CAD97 commented Apr 24, 2023

By having a const Ord bound, cmp can never return different values for the same inputs, thereby allowing optimizations like unguarded insertion (where you compare against the firsts element first, and then use the result of comparing the to-be-inserted element to the other elements as the only loop condition to shift the element in the right place. Very efficient, but currently not possible in safe Rust).

You only need a formal guarantee that comparison is pure/consistent if that condition being true is necessary in order to fulfill some unsafe precondition, and failure to uphold sort order consistency could result in unsoundness/UB and not just logic errors and/or "safe misbehavior1". The Ord trait already has a safe requirement that comparison is consistent. Violating that contract will result in standard library routines misbehaving.

As an extra note, simply k#pure Ord isn't strictly enough either, if you ever call other code with access to &T, since T could encapsulate some shared mutability.

std already contains the pattern for this with unsafe trait TrustedLen: Iterator, which refines trait Iterator with a trustable guarantee that Iterator::size_hint is accurate. An on-pain-of-UB guarantee of Ord implementing a consistent total order should similarly be provided by unsafe trait TrustedOrd: Ord, not by const Ord. If having a separate trait for it is too heavy, we could also consider allowing using T: unsafe Trait like T: const Trait works2, which makes the trait bound trustable as an unsafe impl.

Footnotes

  1. Defining "safe misbehavior" in a satisfactory way is an open question.

  2. I'm personally not fully up-to-date on how/why "keyword generic" traits opt into being able to accept a keyword being applied to them. unsafe Trait would absolutely require an opt-in on the trait before being permitted, though, since it makes documentation (i.e. of the trait requirements/guarantees) much more impactful than for safe traits.

@RalfJung
Copy link
Member

RalfJung commented Jul 19, 2023

So what is our plan with this RFC? It looks like the lang team would prefer to have this discussion as part of a concrete motivating feature. What could that be?

  • For float, I made their const behavior part of a recent Pre-RFC. The Pre-RFC suggests to allow compile-time evaluation to make arbitrary choices whenever float operation would be non-deterministic at runtime (i.e., whenever a NaN is produced).
  • For const_eval_select, I guess that would be a separate RFC?
  • And then we have align_offset and align_to. If we specify those to guarantee best possible alignment at runtime but not at compiletime, that would basically let one implement const_eval_select.

To me it seems like a dedicated "safe const_eval_select" RFC would be the best option.

@Noratrieb
Copy link
Member Author

Just looking at the backlinks above, this seems to solve many problems that we're having right now. Among them:

I think accepting this RFC today would have significant benefit already, even without very design-heavy work like designing the API for stable const_eval_select. I don't see a downside to accepting this RFC, it seems clear to me that we want to allow this behavior. Not accepting it risks that future discussions will have to bring up the points here again, instead of just accepting it, and it will unblock things that are already designed today (like const_cstr_from_ptr).

@RalfJung
Copy link
Member

RalfJung commented Feb 25, 2024

rust-lang/rust#121201 requires it

Not quite; only doing that plus also exposing align_offset to const fn without subtle soundness requirements needs it. But life would definitely be a lot easier if we accepted this RFC.

rust-lang/rust#113219 is blocked on it

Is it? The issue you link just says that it uses const_eval_select, not that it uses it in a way that violates the "both cases must behave the same way" requirement. It's not clear to me why this RFC was added to the blocking list for that tracking issue, it may have been a mistake.

But to add to your list, the soundness requirement in const_eval_select that this RFC aims to lift is the reason why assert_unsafe_precondition has a strange requirement that debug_assert_nounwind does not have, and a bunch of review time and docs effort is spent on keeping that all straight.

@RalfJung
Copy link
Member

RalfJung commented Feb 25, 2024

Quoting from this discussion

This is the first time we stabilize a usage of const_eval_select that is not just changing assertions. While it is absolutely benign here (basically inlining a naive strlen impl for const eval), this means if we'd remove const_eval_select, we'd need to either take a perf hit in the runtime code by using the same code or make strlen a lang item.

then we should "stabilize" const_eval_select as a perma-unstable libs function, to affirm that no matter whether it is exposed or not to end users it would still exist in the standard library, if such a thing is desirable.

Maybe the RFC should be re-phrased as being specifically about having a const-eval-select mechanism (without the "identical behavior" restriction) internally and using it from stable const fn? That would make it more concrete, which might alleviate the concern t-lang had last time we tried to push this.

@Noratrieb
Copy link
Member Author

@rust-lang/lang would you be interested in that?

@RalfJung
Copy link
Member

RalfJung commented Feb 25, 2024

@Nilstrieb I wouldn't hold my breadth to get an answer like this. :) There's a reason we nominate issues for them with well-prepared summary comments -- just pinging doesn't work as they are all drowning in notifications.

If you have the time I'd suggest you work on rewording the RFC. I'd be happy to give feedback. Maybe do it in hackmd so collaborative editing becomes easier. The important point is to make sure that all concerns they raised last time are resolved.

@RalfJung
Copy link
Member

Also: Given that this already has >100 comments, making this a new RFC might be better than updating this one. But ofc if there are important points that were raised in this thread they should be carried over into the new RFC.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 8, 2024
…cuviper

align_offset, align_to: no longer allow implementations to spuriously fail to align

For a long time, we have allowed `align_offset` to fail to compute a properly aligned offset, and `align_to` to return a smaller-than-maximal "middle slice". This was done to cover the implementation of `align_offset` in const-eval and Miri. See rust-lang#62420 for more background. For about the same amount of time, this has caused confusion and surprise, where people didn't realize they have to write their code to be defensive against `align_offset` failures.

Another way to put this is: the specification is effectively non-deterministic, and non-determinism is hard to test for -- in particular if the implementation everyone uses to test always produces the same reliable result, and nobody expects it to be non-deterministic to begin with.

With rust-lang#117840, Miri has stopped making use of this liberty in the spec; it now always behaves like rustc. That only leaves const-eval as potential motivation for this behavior. I do not think this is sufficient motivation. Currently, none of the relevant functions are stably const: `align_offset` is unstably const, `align_to` is not const at all. I propose that if we ever want to make these const-stable, we just accept the fact that they can behave differently at compile-time vs at run-time. This is not the end of the world, and it seems to be much less surprising to programmers than unexpected non-determinism. (Related: rust-lang/rfcs#3352.)

`@thomcc` has repeatedly made it clear that they strongly dislike the non-determinism in align_offset, so I expect they will support this. `@oli-obk,` what do you think? Also, whom else should we involve? The primary team responsible is clearly libs-api, so I will nominate this for them. However, allowing const-evaluated code to behave different from run-time code is t-lang territory. The thing is, this is not stabilizing anything t-lang-worthy immediately, but it still does make a decision we will be bound to: if we accept this change, then
- either `align_offset`/`align_to` can never be called in const fn,
- or we allow compile-time behavior to differ from run-time behavior.

So I will nominate for t-lang as well, with the question being: are you okay with accepting either of these outcomes (without committing to which one, just accepting that it has to be one of them)? This closes the door to "have `align_offset` and `align_to` at compile-time and also always have compile-time behavior match run-time behavior".

Closes rust-lang#62420
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 8, 2024
Rollup merge of rust-lang#121201 - RalfJung:align_offset_contract, r=cuviper

align_offset, align_to: no longer allow implementations to spuriously fail to align

For a long time, we have allowed `align_offset` to fail to compute a properly aligned offset, and `align_to` to return a smaller-than-maximal "middle slice". This was done to cover the implementation of `align_offset` in const-eval and Miri. See rust-lang#62420 for more background. For about the same amount of time, this has caused confusion and surprise, where people didn't realize they have to write their code to be defensive against `align_offset` failures.

Another way to put this is: the specification is effectively non-deterministic, and non-determinism is hard to test for -- in particular if the implementation everyone uses to test always produces the same reliable result, and nobody expects it to be non-deterministic to begin with.

With rust-lang#117840, Miri has stopped making use of this liberty in the spec; it now always behaves like rustc. That only leaves const-eval as potential motivation for this behavior. I do not think this is sufficient motivation. Currently, none of the relevant functions are stably const: `align_offset` is unstably const, `align_to` is not const at all. I propose that if we ever want to make these const-stable, we just accept the fact that they can behave differently at compile-time vs at run-time. This is not the end of the world, and it seems to be much less surprising to programmers than unexpected non-determinism. (Related: rust-lang/rfcs#3352.)

`@thomcc` has repeatedly made it clear that they strongly dislike the non-determinism in align_offset, so I expect they will support this. `@oli-obk,` what do you think? Also, whom else should we involve? The primary team responsible is clearly libs-api, so I will nominate this for them. However, allowing const-evaluated code to behave different from run-time code is t-lang territory. The thing is, this is not stabilizing anything t-lang-worthy immediately, but it still does make a decision we will be bound to: if we accept this change, then
- either `align_offset`/`align_to` can never be called in const fn,
- or we allow compile-time behavior to differ from run-time behavior.

So I will nominate for t-lang as well, with the question being: are you okay with accepting either of these outcomes (without committing to which one, just accepting that it has to be one of them)? This closes the door to "have `align_offset` and `align_to` at compile-time and also always have compile-time behavior match run-time behavior".

Closes rust-lang#62420
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Mar 9, 2024
align_offset, align_to: no longer allow implementations to spuriously fail to align

For a long time, we have allowed `align_offset` to fail to compute a properly aligned offset, and `align_to` to return a smaller-than-maximal "middle slice". This was done to cover the implementation of `align_offset` in const-eval and Miri. See rust-lang/rust#62420 for more background. For about the same amount of time, this has caused confusion and surprise, where people didn't realize they have to write their code to be defensive against `align_offset` failures.

Another way to put this is: the specification is effectively non-deterministic, and non-determinism is hard to test for -- in particular if the implementation everyone uses to test always produces the same reliable result, and nobody expects it to be non-deterministic to begin with.

With rust-lang/rust#117840, Miri has stopped making use of this liberty in the spec; it now always behaves like rustc. That only leaves const-eval as potential motivation for this behavior. I do not think this is sufficient motivation. Currently, none of the relevant functions are stably const: `align_offset` is unstably const, `align_to` is not const at all. I propose that if we ever want to make these const-stable, we just accept the fact that they can behave differently at compile-time vs at run-time. This is not the end of the world, and it seems to be much less surprising to programmers than unexpected non-determinism. (Related: rust-lang/rfcs#3352.)

`@thomcc` has repeatedly made it clear that they strongly dislike the non-determinism in align_offset, so I expect they will support this. `@oli-obk,` what do you think? Also, whom else should we involve? The primary team responsible is clearly libs-api, so I will nominate this for them. However, allowing const-evaluated code to behave different from run-time code is t-lang territory. The thing is, this is not stabilizing anything t-lang-worthy immediately, but it still does make a decision we will be bound to: if we accept this change, then
- either `align_offset`/`align_to` can never be called in const fn,
- or we allow compile-time behavior to differ from run-time behavior.

So I will nominate for t-lang as well, with the question being: are you okay with accepting either of these outcomes (without committing to which one, just accepting that it has to be one of them)? This closes the door to "have `align_offset` and `align_to` at compile-time and also always have compile-time behavior match run-time behavior".

Closes rust-lang/rust#62420
@Noratrieb
Copy link
Member Author

#3514 already integrates the most important part and we've just changed the internal rules for const eval select anyways to make it safe. So I'm gonna say "task failed successfully" and close the RFC.
I still don't understand why this wasn't just accepted, there seemed to be consensus that it's good 🤷

@Noratrieb Noratrieb closed this May 2, 2024
@RalfJung
Copy link
Member

RalfJung commented May 2, 2024 via email

@tarcieri
Copy link

tarcieri commented May 2, 2024

Is there presently another issue for tracking stabilization of const eval select? Or should we just wait for a prospective stabilization RFC like @RalfJung is suggesting?

@RalfJung
Copy link
Member

RalfJung commented May 2, 2024

I have created a tracking issue: rust-lang/rust#124625

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Proposals relating to compile time evaluation (CTFE). T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.