-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Made some functions in time module const #50545
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to have some tests for these functions showing various conversions between different duration units
src/libcore/time.rs
Outdated
@@ -204,7 +204,7 @@ impl Duration { | |||
/// [`subsec_nanos`]: #method.subsec_nanos | |||
#[stable(feature = "duration", since = "1.3.0")] | |||
#[inline] | |||
pub fn as_secs(&self) -> u64 { self.secs } | |||
pub const fn as_secs(&self) -> u64 { self.secs } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add #[rustc_const_unstable="duration_getters"]
to the newly const-ed functions so they aren't immediately constant on the next stable rust?
Oh... I should have explained better. Could you add some tests that do this kind of conversion at compile-time? So e.g. use it for computing an array length or something weird like that. |
That makes sense. I'l try to come up with something.Also, where should I put these tests? |
I usually throw them into https://github.com/rust-lang/rust/tree/master/src/test/ui/const-eval as a |
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
// compile-pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this test passing without a feature attribute?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for that, it should not pass. I will update PR
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
OK Lgtm now, please squash the commits down to one, then we can send it off to bors |
added rustc_const_unstable attribute extended tests added conversion test fixed tidy test added feature attribute
@bors r+ |
📌 Commit 4d8d0a6 has been approved by |
@bors rollup This is not gonna conflict with anything |
Made some functions in time module const They may be const, or i missed something?
Made some functions in time module const They may be const, or i missed something?
Made some functions in time module const They may be const, or i missed something?
Rollup of 13 pull requests Successful merges: - #50544 (Cleanup some dependencies) - #50545 (Made some functions in time module const) - #50550 (use fmt::Result where applicable) - #50558 (Remove all reference to DepGraph::work_products) - #50602 (Update canonicalize docs) - #50607 (Allocate Symbol strings from an arena) - #50613 (Migrate the toolstate update bot to rust-highfive) - #50624 (fs::write: Add example writing a &str) - #50634 (Do not silently truncate offsets for `read_at`/`write_at` on emscripten) - #50644 (AppVeyor: Read back trace from crash dump on failure.) - #50661 (Ignore non .rs files for tidy libcoretest) - #50663 (rustc: Allow an edition's feature on that edition) - #50667 (rustc: Only suggest deleting `extern crate` if it works) Failed merges:
They may be const, or i missed something?