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

Tracking Issue for constify-ing non-trait Duration methods #72440

Closed
3 of 5 tasks
marmeladema opened this issue May 21, 2020 · 15 comments · Fixed by #131289
Closed
3 of 5 tasks

Tracking Issue for constify-ing non-trait Duration methods #72440

marmeladema opened this issue May 21, 2020 · 15 comments · Fixed by #131289
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-time Area: Time C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@marmeladema
Copy link
Contributor

marmeladema commented May 21, 2020

This is a tracking issue for constify-ing non-trait Duration methods

The feature gates for the issueis #![feature(duration_consts_float)]. The methods covered by that are:

  • Duration::as_secs_f64
  • Duration::as_secs_f32
  • Duration::as_millis_f64
  • Duration::as_millis_f32
  • Duration::div_duration_f64
  • Duration::div_duration_f32

Steps

Unresolved Questions

XXX --- list all the "unresolved questions" found in the RFC to ensure they are
not forgotten

Implementation history

@marmeladema marmeladema added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label May 21, 2020
@jonas-schievink jonas-schievink added A-const-fn T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels May 21, 2020
@marmeladema
Copy link
Contributor Author

I also would like to add a Duration::MAX associated const. Should it be part of the same tracking issue?

@Mark-Simulacrum
Copy link
Member

I don't think so -- adding a new constant seems pretty orthogonal to me.

@marmeladema
Copy link
Contributor Author

I don't think so -- adding a new constant seems pretty orthogonal to me.

It's orthogonal but depends on Duration::new being const so I thought having a tracking issue just for one associated const might be overkill.

@RalfJung
Copy link
Member

RalfJung commented May 22, 2020

Because they end up calling f32|64::is_finite which ultimately call mem::transmute which is not const.

FWIW, #72449 makes the other safe transmute wrappers const. Once that lands, abs_private and is_finite could also be made const.

@RalfJung
Copy link
Member

All right, #72449 will make is_finite const as well, so you should be able to use that.

@marmeladema
Copy link
Contributor Author

Great! I'll wait for #72449 to land and update my PR accordingly.

marmeladema added a commit to marmeladema/rust that referenced this issue May 30, 2020
@RalfJung
Copy link
Member

The tracking issue should also notice that stabilizing any of the f32/f64 methods is blocked on stabilizing floating point in const at all (#57241).

@marmeladema marmeladema changed the title Tracking Issue for constify-ing most non-trait Duration methods Tracking Issue for constify-ing non-trait Duration methods May 30, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jul 14, 2020
…i-obk

Constify most non-trait `Duration` methods as described in rust-lang#72440

The remaining methods could probably be made const once rust-lang#72449 lands with support for `f<32|64>::is_finite()`.
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 14, 2020
…i-obk

Constify most non-trait `Duration` methods as described in rust-lang#72440

The remaining methods could probably be made const once rust-lang#72449 lands with support for `f<32|64>::is_finite()`.
marmeladema added a commit to marmeladema/rust that referenced this issue Jul 15, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 16, 2020
Constify most non-trait `Duration` methods as described in rust-lang#72440

The remaining methods could probably be made const once rust-lang#72449 lands with support for `f<32|64>::is_finite()`.
@KodrAus KodrAus added the Libs-Tracked Libs issues that are tracked on the team's project board. label Jul 29, 2020
CDirkx added a commit to CDirkx/rust that referenced this issue Sep 12, 2020
Make the following methods of `Duration` unstable const under `duration_const_2`:
 - `from_secs_f64`
 - `from_secs_f32`
 - `mul_f64`
 - `mul_f32`
 - `div_f64`
 - `div_f32`

This results in all methods of `Duration` being (unstable) const.

Also adds tests for these methods in a const context, moved the test to `library` as part of rust-lang#76268.

Possible because of rust-lang#72449, which made the relevant `f32` and `f64` methods const.

Tracking issue: rust-lang#72440
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 12, 2020
…orse

Make all methods of `Duration` unstably const

Make the following methods of `Duration` unstable const under `duration_const_2`:
 - `from_secs_f64`
 - `from_secs_f32`
 - `mul_f64`
 - `mul_f32`
 - `div_f64`
 - `div_f32`

This results in all methods of `Duration` being (unstable) const.

Moved the tests to `library` as part of rust-lang#76268.

Possible because of rust-lang#72449, which made the relevant `f32` and `f64` methods const.

Tracking issue: rust-lang#72440

r? @ecstatic-morse
RalfJung added a commit to RalfJung/rust that referenced this issue Sep 13, 2020
…orse

Make all methods of `Duration` unstably const

Make the following methods of `Duration` unstable const under `duration_const_2`:
 - `from_secs_f64`
 - `from_secs_f32`
 - `mul_f64`
 - `mul_f32`
 - `div_f64`
 - `div_f32`

This results in all methods of `Duration` being (unstable) const.

Moved the tests to `library` as part of rust-lang#76268.

Possible because of rust-lang#72449, which made the relevant `f32` and `f64` methods const.

Tracking issue: rust-lang#72440

r? @ecstatic-morse
RalfJung added a commit to RalfJung/rust that referenced this issue Sep 13, 2020
…orse

Make all methods of `Duration` unstably const

Make the following methods of `Duration` unstable const under `duration_const_2`:
 - `from_secs_f64`
 - `from_secs_f32`
 - `mul_f64`
 - `mul_f32`
 - `div_f64`
 - `div_f32`

This results in all methods of `Duration` being (unstable) const.

Moved the tests to `library` as part of rust-lang#76268.

Possible because of rust-lang#72449, which made the relevant `f32` and `f64` methods const.

Tracking issue: rust-lang#72440

r? @ecstatic-morse
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 16, 2020
…orse

Make all methods of `Duration` unstably const

Make the following methods of `Duration` unstable const under `duration_const_2`:
 - `from_secs_f64`
 - `from_secs_f32`
 - `mul_f64`
 - `mul_f32`
 - `div_f64`
 - `div_f32`

This results in all methods of `Duration` being (unstable) const.

Moved the tests to `library` as part of rust-lang#76268.

Possible because of rust-lang#72449, which made the relevant `f32` and `f64` methods const.

Tracking issue: rust-lang#72440

r? @ecstatic-morse
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Oct 24, 2022
…on-try-from-secs-float, r=dtolnay

Stabilize `duration_checked_float`

## Stabilization Report

This stabilization report is for a stabilization of `duration_checked_float`, tracking issue: rust-lang#83400.

### Implementation History

- rust-lang#82179
- rust-lang#90247
- rust-lang#96051
- Changed error type to `FromFloatSecsError` in rust-lang#90247
- rust-lang#96051 changes the rounding mode to round-to-nearest instead of truncate.

## API Summary

This stabilization report proposes the following API to be stabilized in `core`, along with their re-exports in `std`:

```rust
// core::time

impl Duration {
    pub const fn try_from_secs_f32(secs: f32) -> Result<Duration, TryFromFloatSecsError>;
    pub const fn try_from_secs_f64(secs: f64) -> Result<Duration, TryFromFloatSecsError>;
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct TryFromFloatSecsError { ... }

impl core::fmt::Display for TryFromFloatSecsError { ... }
impl core::error::Error for TryFromFloatSecsError { ... }
```

These functions are made const unstable under `duration_consts_float`, tracking issue rust-lang#72440.

There is an open question in the tracking issue around what the error type should be called which I was hoping to resolve in the context of an FCP.

In this stabilization PR, I have altered the name of the error type to `TryFromFloatSecsError`. In my opinion, the error type shares the name of the method (adjusted to accommodate both types of floats), which is consistent with other error types in `core`, `alloc` and `std` like `TryReserveError` and `TryFromIntError`.

## Experience Report

Code such as this is ready to be converted to a checked API to ensure it is panic free:

```rust
impl Time {
    pub fn checked_add_f64(&self, seconds: f64) -> Result<Self, TimeError> {
        // Fail safely during `f64` conversion to duration
        if seconds.is_nan() || seconds.is_infinite() {
            return Err(TzOutOfRangeError::new().into());
        }

        if seconds.is_sign_positive() {
            self.checked_add(Duration::from_secs_f64(seconds))
        } else {
            self.checked_sub(Duration::from_secs_f64(-seconds))
        }
    }
}
```

See: artichoke/artichoke#2194.

`@rustbot` label +T-libs-api -T-libs

cc `@mbartlett21`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Oct 24, 2022
…on-try-from-secs-float, r=dtolnay

Stabilize `duration_checked_float`

## Stabilization Report

This stabilization report is for a stabilization of `duration_checked_float`, tracking issue: rust-lang#83400.

### Implementation History

- rust-lang#82179
- rust-lang#90247
- rust-lang#96051
- Changed error type to `FromFloatSecsError` in rust-lang#90247
- rust-lang#96051 changes the rounding mode to round-to-nearest instead of truncate.

## API Summary

This stabilization report proposes the following API to be stabilized in `core`, along with their re-exports in `std`:

```rust
// core::time

impl Duration {
    pub const fn try_from_secs_f32(secs: f32) -> Result<Duration, TryFromFloatSecsError>;
    pub const fn try_from_secs_f64(secs: f64) -> Result<Duration, TryFromFloatSecsError>;
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct TryFromFloatSecsError { ... }

impl core::fmt::Display for TryFromFloatSecsError { ... }
impl core::error::Error for TryFromFloatSecsError { ... }
```

These functions are made const unstable under `duration_consts_float`, tracking issue rust-lang#72440.

There is an open question in the tracking issue around what the error type should be called which I was hoping to resolve in the context of an FCP.

In this stabilization PR, I have altered the name of the error type to `TryFromFloatSecsError`. In my opinion, the error type shares the name of the method (adjusted to accommodate both types of floats), which is consistent with other error types in `core`, `alloc` and `std` like `TryReserveError` and `TryFromIntError`.

## Experience Report

Code such as this is ready to be converted to a checked API to ensure it is panic free:

```rust
impl Time {
    pub fn checked_add_f64(&self, seconds: f64) -> Result<Self, TimeError> {
        // Fail safely during `f64` conversion to duration
        if seconds.is_nan() || seconds.is_infinite() {
            return Err(TzOutOfRangeError::new().into());
        }

        if seconds.is_sign_positive() {
            self.checked_add(Duration::from_secs_f64(seconds))
        } else {
            self.checked_sub(Duration::from_secs_f64(-seconds))
        }
    }
}
```

See: artichoke/artichoke#2194.

``@rustbot`` label +T-libs-api -T-libs

cc ``@mbartlett21``
@MoSal
Copy link

MoSal commented Apr 27, 2023

Didn't see this mentioned anywhere with a quick search, but Duration::from_secs_f64 is not const anymore?

Is #![feature(duration_consts_float)] no-op now?

@MoSal
Copy link

MoSal commented Apr 29, 2023

Didn't see this mentioned anywhere with a quick search, but Duration::from_secs_f64 is not const anymore?

Is #![feature(duration_consts_float)] no-op now?

Looking at this closer today, the from_* and try_from_* float functions were no longer
const after #110393. That's because the try_from_secs macro makes use of $int_ty::from($expr), the trait impls of which are no longer const.

The from calls can be replaced with ($expr as $int_ty) if it is desired to restore the const impls.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 14, 2024
…, r=scottmcm

Implement `Duration::as_millis_{f64,f32}`

Implementation of rust-lang#122451.

Linked const-unstability to rust-lang#72440, so the post there should probably be updated to mentions the 2 new methods when/if this PR is merged.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 15, 2024
Rollup merge of rust-lang#122479 - GrigorenkoPV:duration_millis_float, r=scottmcm

Implement `Duration::as_millis_{f64,f32}`

Implementation of rust-lang#122451.

Linked const-unstability to rust-lang#72440, so the post there should probably be updated to mentions the 2 new methods when/if this PR is merged.
@RalfJung
Copy link
Member

RalfJung commented Sep 8, 2024

@rust-lang/libs-api With #57241 being stable now, I think we can proceed with stabilizing these as well? :)

EDIT: I updated the function list; some things listed there were actually not even unstably const any more.

@RalfJung RalfJung added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Sep 8, 2024
@rfcbot
Copy link

rfcbot commented Sep 9, 2024

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.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Sep 9, 2024
@dtolnay dtolnay removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Sep 9, 2024
@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Oct 1, 2024
@rfcbot
Copy link

rfcbot commented Oct 1, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Oct 1, 2024
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Oct 11, 2024
@rfcbot
Copy link

rfcbot commented Oct 11, 2024

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

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@bors bors closed this as completed in 3e16b77 Oct 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 12, 2024
Rollup merge of rust-lang#131289 - RalfJung:duration_consts_float, r=tgross35

stabilize duration_consts_float

Waiting for FCP in rust-lang#72440 to pass.

`as_millis_f32` and `as_millis_f64` are not stable at all yet, so I moved their const-stability together with their regular stability (tracked at rust-lang#122451).

Fixes rust-lang#72440
RalfJung pushed a commit to RalfJung/miri that referenced this issue Oct 14, 2024
stabilize duration_consts_float

Waiting for FCP in rust-lang/rust#72440 to pass.

`as_millis_f32` and `as_millis_f64` are not stable at all yet, so I moved their const-stability together with their regular stability (tracked at rust-lang/rust#122451).

Fixes rust-lang/rust#72440
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Oct 17, 2024
@RalfJung RalfJung added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-time Area: Time C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants