rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.69k stars 12.75k forks source link

Tracking issue for a minimal subset of RFC 911, const fn #53555

Closed Centril closed 6 years ago

Centril commented 6 years ago

This is a tracking issue for the RFC "Const functions and inherent methods" (rust-lang/rfcs#911).

This issue only tracks a minimal subset of the proposal in 911 that we are (hopefully) comfortable with stabilizing. To opt into the minimal subset, use #![feature(min_const_fn)]. To use the more expansive feature set, you can continue using #![feature(const_fn)] and other associated feature gates.

The minimal set will not include items from the following (incomplete) list:

  1. const fns with type parameters with bounds (including where clauses) in scope (including from the parent e.g. impl) other than: lifetimes, Sized, or (the "un"-bound) ?Sized

    This restriction exists because we are not sure about our story around what bounds mean in a const fn context. See https://github.com/rust-lang/rfcs/pull/2237, https://github.com/rust-rfcs/const-eval/issues/1, and https://github.com/Centril/rfc-effects/ for a discussion on this.

  2. const fns with argument types or return types that contain fn pointers, dyn Trait, or impl Trait.

    This is checked recursively. The restriction ensures that you may not reach a value of these types by any means.

    This restriction exists for the same reasons as in 1.

  3. const fns with any operations on floating-point numbers. This is achieved by making any floating-point operation not be const inside const fn.

    This restriction exists because we are not sure about the story wrt. determinism, achieving the same results on compile-time / run-time (including other machines) and floating points.

  4. using a const fn call in a pattern, e.g.;

    const fn twice<T: Copy>(x: T) -> (T, T) { (x, x) }
    match x {
        twice(21) => { ... }
        _ => { ... }
    }
  5. anything else that is not currently in const_fn or constants

    • raw ptr to usize cast (e.g. *const/mut T -> usize).
    • raw ptr deref.
    • if / if let / match.
    • loop / while.
    • let and destructuring.
  6. union field access.

  7. code requiring unsafe blocks.

Exhaustive list of features supported in const fn with #![feature(min_const_fn)]:

  1. type parameters where the parameters have any of the following as part of their bounds (either on where or directly on the parameters):

    1. lifetimes
    2. Sized

    This means that <T: 'a + ?Sized> and <T: 'b + Sized> + <T> are all permitted. Note that ?Sized is the absence of a constraint when bounds have been fully elaborated which includes adding implicit Sized bounds. This entails that permitting Sized + lifetimes allows the above examples.

    This rule also applies to type parameters of items that contain const fns.

  2. arithmetic operators on integers

  3. boolean operators (except for && and || which are banned since they are short-circuiting).

  4. any kind of aggregate constructor (array, struct, enum, tuple, ...)

  5. calls to other const fns (methods and functions)

  6. index operations on arrays and slices

  7. field accesses on structs and tuples

  8. reading from constants (but not statics, not even taking a reference to a static)

  9. & and * (only dereferencing of references, not raw pointers)

  10. casts except for raw pointer to usize casts

  11. const unsafe fn is allowed, but the body must consist of safe operations only

The bar for stabilizing const fns in libcore/liballoc/libstd will be that they are writable in stable user code (unless they are wrappers for intrinsics, i.e. size_of and align_of). This means that they must work with min_const_fn.

Things to be done before stabilizing:

Unresolved questions:

None.

Vocabulary:

oli-obk commented 6 years ago

similar to how it works with generic functions. They end up in metadata and are instantiated where required.

SimonSapin commented 6 years ago
  1. Can be solved by removing the bounds from the type and adding it to impls only

Yes, but that means anyone can now construct a value (by calling the constructor) that doesn’t make sense and doesn’t have the usual methods. Maybe this is worth the trade-off, but it is a semantic change.

does const fn foo<T: Clone>(...) { .. } mean that T has a "const implementation" of Clone or is any current clone implementation permissible?

I assume that we’ll find use cases for both, and so we’ll need syntax for opting into one of them. I think the one that is opt-in should be the one that is new, which is the former ("const impl" of Clone). So the current syntax should be allowed to expressed the latter (any current Clone implementation is premissible).

or must it be a const fn() -> u8

Same, you even used new syntax yourself.

Centril commented 6 years ago

@SimonSapin

Maybe this is worth the trade-off, but it is a semantic change.

If it's an internal API maybe y'all can cope with that? But the situation is not ideal, I agree -- hopefully this will eventually change :)

I assume that we’ll find use cases for both, and so we’ll need syntax for opting into one of them. I think the one that is opt-in should be the one that is new, which is the former ("const impl" of Clone). So the current syntax should be allowed to expressed the latter (any current Clone implementation is premissible).

I think I agree with this standpoint, also because I think it is a cleaner solution. However, it will have costs to ergonomics for sure; Particularly if we expect const fn to be extremely common (even more common than fn is eventually if everything goes right...), which I'm hoping it will. However; this is all very much "needs RFC and very deliberate design work" territory; so unfortunately this will have to wait until we agree on a design.

Same, you even used new syntax yourself.

Well; another design is that you must explicitly write const fn foo(bar: const fn() -> u8) if you want to be able to call bar inside foo and that const fn foo(bar: fn() -> u8) means that bar is a non-const fn.

oli-obk commented 6 years ago

I think the one that is opt-in should be the one that is new, which is the former ("const impl" of Clone). So the current syntax should be allowed to expressed the latter (any current Clone implementation is premissible).

That would require bounds like T: const Clone or something of the kind. I'm all for this, but the discussion around this is what has stalled the const_fn tracking issue into a grinding halt. Thus we're not discussing it for min_const_fn. I propose that once min_const_fn has been merged, we'll create an RFC/tracking issue that stabilizes function pointers and trait bounds in const fns and forces any future desire to call function pointers in const fns to use some other syntax.

The restriction that T: Clone does not allow us to call clone on values of type T inside const fn is what the const_fn feature already implements, so the stabilization of that requires no further implementation.

TimDiekmann commented 6 years ago

Well; another design is that you must explicitly write const fn foo(bar: const fn() -> u8) if you want to be able to call bar inside foo and that const fn foo(bar: fn() -> u8) means that bar is a non-const fn.

Does this mean you can pass a non-const fn to a const fn as long as you don't call it?

Centril commented 6 years ago

@TimDiekmann yes, that would be the implication of that design.

oli-obk commented 6 years ago

We're getting off topic here I think. This issue is about as few features as we can aggree on. Let's take any further discussion into an extra issue that talks about adding function pointes and trait bounds to const fns (or the already existing tracking issues for control flow, let bindings, ...)

SimonSapin commented 6 years ago

Particularly if we expect const fn to be extremely common (even more common than fn is eventually if everything goes right...), which I'm hoping it will.

Isn’t it already too late to make const the default, and non-constness require opt-in?

Thus we're not discussing it for min_const_fn

That’s fair. As mentioned I’m all for incremental progress over blocking everything. I’m only hoping we can find a second slightly-larger subset of functionality “relatively quickly” after we stabilize min_const_fn.

Does this mean you can pass a non-const fn to a const fn as long as you don't call it?

This is the use case I’ve described (for trivial constructors).

RalfJung commented 6 years ago

FTR, the issue for all things related to generics, function pointers and trait objects in const fn is https://github.com/rust-rfcs/const-eval/issues/1

rfcbot commented 6 years ago

The final comment period, with a disposition to merge, as per the review above, is now complete.

therealprof commented 6 years ago

Is there any plan or timeframe for stabilisation of this feature?

This is blocking a major embedded use case to safely pass data and peripherals in and out of interrupt handlers as far as I can see (cf. https://github.com/japaric/bare-metal/issues/11). The use of const fn is currently cfg-gated to make the crate work in stable (without static Mutexes in that case) but min_const_fn would suffice to make that IMHO important feature generally available.

Centril commented 6 years ago

@therealprof Stabilization is blocked on merging https://github.com/rust-lang/rust/pull/53851.

gnzlbg commented 6 years ago

This restriction exists because we are not sure about the story wrt. determinism, achieving the same results on compile-time / run-time (including other machines) and floating points.

We discussed this on IRC a while ago, but I don't see it mentioned here, so it might be worth it to update the OP. We are sure about this: it is, in general, impossible to achieve the same results at compile-time and run-time (on all targets). What const fn should do? Probably just implement IEEE arithmetic, fixing the implementation-defined behavior to some behavior in a target independent way. cc @rkruppe

RalfJung commented 6 years ago

As uncovered in https://github.com/rust-lang/rust/issues/54696, raw pointer comparison is not the only concern -- we also have ti disallow function pointer comparison as CTFE does not support that operation. Does min_const_fn handle that?

oli-obk commented 6 years ago

Well... we do have "function pointers in const fn are unstable" even for casting functions to function pointers.

RalfJung commented 6 years ago

But we could still write a function that takes two fn ptrs and compares them? It couldn't be called (currently), but we should rule out the binop as well.

cramertj commented 6 years ago

@RalfJung Attempting to define

const fn cmp(x: fn(), y: fn()) -> bool {
    x == y
}

with #![feature(min_const_fn)] gives "error: function pointers in const fn are unstable".

RalfJung commented 6 years ago

If we have that in a test, I am happy :)

Centril commented 6 years ago

@oli-obk given that we landed https://github.com/rust-lang/rust/pull/53851, should we move to stabilize this?

bstrie commented 6 years ago

However, I'd like to see a test ensuring that

   const FOO: NonZeroU8 = unsafe { NonZeroU8::new_unchecked(0) };

is caught by the sanity check. Currently nightly accepts that constant without complaining.

@RalfJung , it would appear that post-#54835 nightly still accepts this code without warning. Did you intend for this code not to work?

oli-obk commented 6 years ago

Nightly on the playground gives me

error[E0080]: this constant likely exhibits undefined behavior
 --> src/main.rs:3:1
  |
3 | const FOO: NonZeroU8 = unsafe { NonZeroU8::new_unchecked(0) };
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1
  |
  = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior

https://play.rust-lang.org/?gist=8544bb6dd824c73bd0ea8466aad29ae9&version=nightly&mode=debug&edition=2015

RalfJung commented 6 years ago

And there's tests as well: https://github.com/rust-lang/rust/blob/master/src/test/ui/consts/const-eval/ub-nonnull.rs

This can been fixed with https://github.com/rust-lang/rust/pull/54762

therealprof commented 6 years ago

Any change we can get this into beta? I'd really love to do some 2018 edition proving and cleanup of some embedded crates but without min_const_fn available on beta, realistic checks are somewhat limited.

oli-obk commented 6 years ago

It will be in the next beta. Until then you could just use nightly without any feature gates

therealprof commented 6 years ago

@oli-obk That's what I'm doing at the moment but I thought the point of the beta was to test Edition features and nightly might behave differently. I guess I'm looking forward then to trying the next beta and everything working fine just like on nighty. ;)

Centril commented 6 years ago

Documentation was done in https://github.com/rust-lang-nursery/reference/pull/440. Since everything is done, I'll close this out.

therealprof commented 6 years ago

@oli-obk > It will be in the next beta. Until then you could just use nightly without any feature gates

# rustc +beta --version
rustc 1.30.0-beta.15 (590121930 2018-10-12)
# cargo +beta build --release
...
error[E0658]: const fn is unstable (see issue #53555)
  --> /Users/egger/OSS/bare-metal/src/lib.rs:22:5
   |
22 | /     pub const unsafe fn new(address: usize) -> Self {
23 | |         Peripheral {
24 | |             address: address as *mut T,
25 | |         }
26 | |     }
   | |_____^
crlf0710 commented 6 years ago

@therealprof "the next beta" means the 1.31 one, which will arrive some time around the time 1.30 stable ships(Oct 25).