Consider never removing `std::⁠thread::sleep_ms` while keeping it deprecated

libs
1

A rare issue programmers have is adding sleep statements for testing purposes then forgetting to remove them. They somehow get missed in review and wind up in a real application leading to a slowdown.

What I love about std::thread::sleep_ms is I can use it for testing purposes and get reminded through compiler warnings about its existence on every build. Then when I'm done, I click all the links on the warnings to find the statements to remove without having to search too hard. I think this is a great feature and perhaps it would be good to never remove this API while keeping it deprecated. I'd be a little sad to see it go.

2

AFAIK, we have never actually removed anything that was stabilized before deprecation. Some folks have proposed that we try to do that across edition boundaries, but there's no agreement yet.

6 Likes
3

Something I use to prevent accidents like this: I diff before I commit, carefully checking each change. I tend to have a lot more temporary debug aids than just sleep.

1 Like
4

Might be a cleaner solution:

#[cfg_attr(not(feature = "allow_dev_artifacts"),
    deprecated = "forgotten to remove development artifact")]
fn sleep_dev(ms: u64) {
    std::thread::sleep(std::time::Duration::from_millis(ms));
}

Furthermore, "remove allow_dev_artifacts from default" should appear in the checklist.

2 Likes
5

For arbitrary bits of code, you could have something like:

#[deprecated = "DO NOT COMMIT"]
macro_rules! dnc {
    ($($t:tt)*) => { $($t)* }
}

fn main() {
    dnc!( /* anything goes! */ );
}

warning: use of deprecated macro `dnc`: DO NOT COMMIT
 --> src/main.rs:7:5
  |
7 |     dnc!( /* anything goes! */ );
  |     ^^^
  |
  = note: `#[warn(deprecated)]` on by default
3 Likes
6

I also diff, but I'm not infallible. My point was how it's nice that I get something screaming at me. Additionally, I don't have to bother importing or specifying std::time::Duration, which is quite verbose for quick changes.

7

That's quite nice and probably more appropriate. Wish it were built in though.

8

Assuming deprecated items start being removed from std, I don't think the justification that some people use deprecation as an ad hoc way to trigger compilation warnings in debug builds is a good reason to keep them around :upside_down_face:

That and there are many more debugging aids (printing, interactivity, sleeping, extra locking...) one might want, so really I'd like to see this written

compile_warning!("debugging!");
std::thread::sleep(std::time::Duration::from_millis(10));
5 Likes
9

Agreed :+1:

A way to do this at the call site and at the declaration would be useful. I've actually been abusing the deprecated attribute in my test code for debugging functionality that shouldn't make it into the comitted test suite.

10

Some prior discussion: Undeprecate `thread::sleep_ms` by matklad · Pull Request #51610 · rust-lang/rust · GitHub

1 Like
11

Although undeprecating this would also break their desired warning here.

12

Note that I've not really seen any discussion about trying to remove things like sleep_ms that are only deprecated for "this works fine but there's another thing that's a bit better".

The discussions I've seen about trying to remove things from an edition are about things are are just massive footguns that nobody should ever use -- notably mem::uninitialized.

6 Likes
13

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.