Add Duration::from_micros #44436

gagath · 2017-09-08T21:04:33Z

This fixes #44400 that explains why it could be useful for embedded designs timing.

rust-highfive · 2017-09-08T21:04:45Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

mattico · 2017-09-08T21:20:34Z

This'll need a feature gate (right?).

You'll need to:

Create a tracking issue (or just use Consider adding a from_micros for std::time::Duration #44400)
~~Add a feature to src/libsyntax/feature_gate.rs~~
~~Add an entry to the unstable book src/doc/unstable-book~~
Add an #[unstable(feature = "", issue = "")] annotation to the function.

sfackler · 2017-09-08T21:28:40Z

src/libsyntax/feature_gate.rs is only used for language features, not library features. I don't think the unstable book needs to be modified anymore either.

SergioBenitez · 2017-09-09T09:11:14Z

src/libstd/time/duration.rs

+    pub fn from_micros(micros: u64) -> Duration {
+        let secs = micros / MICROS_PER_SEC;
+        let nanos = ((micros % MICROS_PER_SEC) as u32) * NANOS_PER_MICRO;
+        Duration {


This should be inlined for consistency.

gagath · 2017-09-09T16:03:45Z

I have commited some changes according to your feedbacks, please let me know if it needs anything else.

kennytm · 2017-09-09T17:13:02Z

src/libstd/time/duration.rs

@@ -114,6 +116,25 @@ impl Duration {
        let secs = millis / MILLIS_PER_SEC;
        let nanos = ((millis % MILLIS_PER_SEC) as u32) * NANOS_PER_MILLI;
        Duration { secs: secs, nanos: nanos }
+


You missed a } here.

gagath · 2017-09-09T22:27:46Z

Thanks @kennytm, fixed!

kennytm · 2017-09-10T06:25:24Z

src/libstd/time/duration.rs

+    #[inline]
+    pub fn from_micros(micros: u64) -> Duration {
+        let secs = micros / MICROS_PER_SEC;
+        let nanos = ((micros % MICROS_PER_SEC) as u32) * NANOS_PER_MICRO;


Typo. Also, the issue must be a number.

[00:02:49] error[E0425]: cannot find value `NANOS_PER_MICRO` in this scope [00:02:49] --> /checkout/src/libstd/time/duration.rs:137:58 [00:02:49] | [00:02:49] 137 | let nanos = ((micros % MICROS_PER_SEC) as u32) * NANOS_PER_MICRO; [00:02:49] | ^^^^^^^^^^^^^^^ did you mean `NANOS_PER_MICROS`? [00:02:49] [00:02:49] error[E0545]: incorrect 'issue' [00:02:49] --> /checkout/src/libstd/time/duration.rs:133:5 [00:02:49] | [00:02:49] 133 | #[unstable(feature = "", issue = "")] [00:02:49] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [00:02:49] [00:02:51] error: aborting due to 2 previous errors [00:02:51] [00:02:51] error: Could not compile `std`.

sfackler · 2017-09-11T21:17:41Z

src/libstd/time/duration.rs

+    /// assert_eq!(1, duration.as_secs());
+    /// assert_eq!(2000, duration.subsec_nanos());
+    /// ```
+    #[unstable(feature = "", issue = "44400")]


The feature can be called whatever you want duration_from_micros is probably fine.

sfackler · 2017-09-11T21:17:51Z

src/libstd/time/duration.rs

+    ///
+    /// # Examples
+    ///
+    /// ```


This test will need to enable that feature to build properly.

sfackler · 2017-09-11T21:18:17Z

Seems like a straightforward/reasonable addition!

@rfcbot fcp merge

rfcbot · 2017-09-11T22:10:24Z

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

No concerns currently listed.

Once these reviewers reach consensus, 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.

gagath · 2017-09-13T11:21:23Z

Thanks for the review @sfackler, I have added a feature name as you suggested and used it in the test.

rfcbot · 2017-09-13T19:29:31Z

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

rfcbot · 2017-09-23T20:01:57Z

The final comment period is now complete.

alexcrichton · 2017-09-23T21:59:47Z

@bors: r+

bors · 2017-09-23T21:59:48Z

📌 Commit b40a9f4 has been approved by alexcrichton

bors · 2017-09-23T22:21:37Z

⌛ Testing commit b40a9f4 with merge 24831c7...

Add Duration::from_micros This fixes #44400 that explains why it could be useful for embedded designs timing.

bors · 2017-09-24T01:51:38Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 24831c7 to master...

Add Duration::from_micros

4db499f

rust-highfive assigned sfackler Sep 8, 2017

SergioBenitez reviewed Sep 9, 2017

View reviewed changes

gagath added 2 commits September 9, 2017 11:46

from_micros: Inlined return for consistency

376837b

from_micros: Added unstable annotation

2e6aed8

kennytm requested changes Sep 9, 2017

View reviewed changes

from_micros: Fix missing {

abc53cc

kennytm reviewed Sep 10, 2017

View reviewed changes

gagath force-pushed the master branch from 9d2fcb8 to a15c541 Compare September 10, 2017 15:38

from_micros: added issue number and fixed typo

a15c541

carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 11, 2017

sfackler reviewed Sep 11, 2017

View reviewed changes

src/libstd/time/duration.rs

///

/// # Examples

///

/// ```

Copy link

Member

sfackler Sep 11, 2017

Choose a reason for hiding this comment

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

This test will need to enable that feature to build properly.

sfackler added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Sep 11, 2017

rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Sep 11, 2017

from_micros: add feature name

b40a9f4

rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Sep 13, 2017

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 Sep 13, 2017

arielb1 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 19, 2017

bors added a commit that referenced this pull request Sep 23, 2017

Auto merge of #44436 - MicroJoe:master, r=alexcrichton

24831c7

Add Duration::from_micros This fixes #44400 that explains why it could be useful for embedded designs timing.

bors merged commit b40a9f4 into rust-lang:master Sep 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Duration::from_micros #44436

Add Duration::from_micros #44436

gagath commented Sep 8, 2017

rust-highfive commented Sep 8, 2017

mattico commented Sep 8, 2017 •

edited

Loading

sfackler commented Sep 8, 2017

SergioBenitez Sep 9, 2017

gagath commented Sep 9, 2017 •

edited

Loading

kennytm Sep 9, 2017

gagath commented Sep 9, 2017

kennytm Sep 10, 2017

sfackler Sep 11, 2017

sfackler Sep 11, 2017

sfackler commented Sep 11, 2017

rfcbot commented Sep 11, 2017 •

edited by BurntSushi

Loading

gagath commented Sep 13, 2017

rfcbot commented Sep 13, 2017

rfcbot commented Sep 23, 2017

alexcrichton commented Sep 23, 2017

bors commented Sep 23, 2017

bors commented Sep 23, 2017

bors commented Sep 24, 2017

Add Duration::from_micros #44436

Add Duration::from_micros #44436

Conversation

gagath commented Sep 8, 2017

rust-highfive commented Sep 8, 2017

mattico commented Sep 8, 2017 • edited Loading

sfackler commented Sep 8, 2017

SergioBenitez Sep 9, 2017

Choose a reason for hiding this comment

gagath commented Sep 9, 2017 • edited Loading

kennytm Sep 9, 2017

Choose a reason for hiding this comment

gagath commented Sep 9, 2017

kennytm Sep 10, 2017

Choose a reason for hiding this comment

sfackler Sep 11, 2017

Choose a reason for hiding this comment

sfackler Sep 11, 2017

Choose a reason for hiding this comment

sfackler commented Sep 11, 2017

rfcbot commented Sep 11, 2017 • edited by BurntSushi Loading

gagath commented Sep 13, 2017

rfcbot commented Sep 13, 2017

rfcbot commented Sep 23, 2017

alexcrichton commented Sep 23, 2017

bors commented Sep 23, 2017

bors commented Sep 23, 2017

bors commented Sep 24, 2017

mattico commented Sep 8, 2017 •

edited

Loading

gagath commented Sep 9, 2017 •

edited

Loading

rfcbot commented Sep 11, 2017 •

edited by BurntSushi

Loading