rust-lang / rust

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

Tracking issue for const `Option` functions #67441

Open 9999years opened 4 years ago

9999years commented 4 years ago

Current candidates with feature = const_option:

impl<T> Option<T> {
    pub const fn as_mut(&mut self) -> Option<&mut T>;
    pub const fn expect(self, msg: &str) -> T;
    pub const fn unwrap(self) -> T;
    pub const unsafe fn unwrap_unchecked(self) -> T;
    pub const fn take(&mut self) -> Option<T>;
    pub const fn replace(&mut self, value: T) -> Option<T>;
}

impl<T> Option<&T> {
    pub const fn copied(self) -> Option<T>
    where
        T: Copy;
}

impl<T> Option<&mut T> {
    pub const fn copied(self) -> Option<T>
    where
        T: Copy;
}

impl<T, E> Option<Result<T, E>> {
    pub const fn transpose(self) -> Result<Option<T>, E>
}

impl<T> Option<Option<T>> {
    pub const fn flatten(self) -> Option<T>;
}

See also the meta-tracking issue for const fns, #57563.

Blocked on:

Also see the corresponding Result tracking issue: https://github.com/rust-lang/rust/issues/82814.

peter-lyons-kehl commented 3 years ago

Yes for constantifying Option.expect.

However, as per https://doc.rust-lang.org/nightly/src/core/option.rs.html, Option.expect(self, msg: &str) calls (private) fn expect_failed(msg: &str), which uses panic!("{}", msg). Using panic! with more than one argument is not const-friendly:

    --> library/core/src/macros/mod.rs:18:38
     |
7    | / macro_rules! panic {
8    | |     () => (
9    | |         $crate::panic!("explicit panic")
10   | |     );
...    |
18   | |         $crate::panicking::panic_fmt($crate::format_args!($fmt, $($arg)+))
     | |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation (#2)
19   | |     );
20   | | }
     | |_- in this expansion of `panic!` (#1)
...
761  | /     macro_rules! format_args {
762  | |         ($fmt:expr) => {{ /* compiler built-in */ }};
763  | |         ($fmt:expr, $($args:tt)*) => {{ /* compiler built-in */ }};
764  | |     }
     | |_____- in this expansion of `$crate::format_args!` (#2)
     | 
    ::: library/core/src/option.rs:1294:5
     |
1294 |       panic!("{}", msg)
     |       ----------------- in this macro invocation (#1)

Since that is not specific to Option, is there any discussion/plan/tracking issue on making panic const-friendly? Isn't it reasonable to assume that any panic is a panic? Or, can panic fail (for example, if formatting fails) and not panic?

memoryruins commented 3 years ago

@peter-kehl the tracking issue for panicking in constants is https://github.com/rust-lang/rust/issues/51999

davidhewitt commented 3 years ago

FYI anyone looking at .unwrap_or - I just naively tried to add this, but given that T may impl Drop, this is currently unsupported.

I wonder if it would be feasible for the compiler to allow use of .unwrap_or for types that do impl drop - e.g. only fail when trying to const-evaluate .unwrap_or for T: impl Drop.

jhpratt commented 3 years ago

@davidhewitt Given that neither const trait impls nor const trait bounds are usable on stable, it would be quite unintuitive if this were. Even on nightly, const traits have yet to be RFC accepted.

davidhewitt commented 3 years ago

Sorry, I typo'd above - I meant to say only allow types which don't impl Drop.

We probably wouldn't want this in the function signature (should just be const fn unwrap_or(default: T)). The compiler could be aware that in some paths inside the function default is disposed of, and so reject substitutions during const evaluation for T which impl Drop.

TBH it sounds messy and slightly leaks information about the function implementation, so I'm unconvinced it's worth the effort. Just musing of a way to allow some forms of const unwrap_or before const impl Trait / const Drop is ready.

jhpratt commented 3 years ago

Yeah, I figured it was a typo and responded accordingly.

I still think it would be awkward to have this magically work for some types but not others, especially if this isn't in the function signature. I'd much rather wait for const trait impls to be RFC approved and land on stable, as that would allow way more than just this. Just look at the blockers on #82814 and realize that having const trait impls would allow ~80% of them (just guessing, I haven't bothered to actually calculate a percentage).

jhpratt commented 3 years ago

If/when #51999 is stabilized, Option::expect and Option::unwrap should be unblocked (I haven't checked this, it's just from reading source code).

CodesInChaos commented 3 years ago

@jhpratt Option::expect passes more than one argument to panic! (and the second argument isn't const), so it'll need more work than the minimal version of const panics considered for stabilization at the moment.

mbartlett21 commented 3 years ago

replace, take, and copied would also be some more that can be done.

Can @9999years add these to the list at the top?

lilasta commented 2 years ago

Option::as_mut has been constified: #89953

CodesInChaos commented 2 years ago

Since the panic now seems to support non-const strings as second argument, Option::expect can now be stabilized, right?

jhpratt commented 2 years ago

@CodesInChaos I believe it's blocked on const_precise_live_drops, along with a number of other methods.

lilasta commented 2 years ago

Option::expect has been constified: #90269

jhpratt commented 2 years ago

unstably for those not looking in further detail ^^

clarfonthey commented 1 year ago

Was there any actual blocker to stabilising Option::unwrap and Option::expect? It's been over a year and I could have sworn they were already stabilised, but I guess not.

jhpratt commented 1 year ago

Precise live drops (or whatever it's called) is the sole blocker to my knowledge.

c410-f3r commented 1 year ago

AFAICT, little progress were made since https://github.com/rust-lang/rust/issues/73255#issuecomment-938086246 so I guess stabilization is likely going to take some time.

bend-n commented 4 months ago

Can we make unwrap_or, ok_or, and, filter, or, or_else, xor, transpose, flatten stable?

Those functions don't seem to have any blockers.

jhpratt commented 4 months ago

filter would need const trait impls and/or const closures, while the rest need const precise live drops.

bend-n commented 4 months ago

forgot the signature of filter, sorry— i didnt realize that the const precise live drops applied to all of the functions, my bad

RalfJung commented 1 month ago

@rust-lang/wg-const-eval how bad of an idea would it be to stabilize at least unwrap and expect with rustc_allow_const_fn_unstable(const_precise_live_drops)?

The blocking concern for const_precise_live_drops is that it may change behavior in the future so it's very hard to make stable guarantees that what builds now, will keep building. But if we only use it internally in core and std that's not so bad -- it's fine for const_precise_live_drops to change behavior as long as it keeps accepting the const-stable functions.

oli-obk commented 1 month ago

Yea that's fine. We will always find a way to support these

RalfJung commented 4 weeks ago

Ah, it's not so easy... rustc_allow_const_fn_unstable(const_precise_live_drops) doesn't actually do anything. :/ It seems that feature flag only has an effect on the crate level.

EDIT: https://github.com/rust-lang/rust/pull/129507 fixes that. Once it is in beta, we can proceed with stabilizing at least unwrap and expect.

RalfJung commented 2 weeks ago

I've cleaned up and updated the list -- some things were listed but they were not even unstably const (such as and, xor, or), others were missing (as_mut, take, replace).

RalfJung commented 19 hours ago

@rust-lang/libs-api with https://github.com/rust-lang/rust/issues/83164 in FCP, I think we can finally stabilize this -- some long-stable fn becoming const fn. Including Option::unwrap which has been requested quite a lot. :)

dtolnay commented 13 hours ago

@rfcbot fcp merge

Everything listed in https://github.com/rust-lang/rust/issues/67441#issue-540694088 (courtesy of Ralf—thank you).

rfcbot commented 13 hours ago

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.