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

Recommend using Once::new() instead of ONCE_INIT? #61746

Closed
estebank opened this issue Jun 11, 2019 · 10 comments · Fixed by #61757
Closed

Recommend using Once::new() instead of ONCE_INIT? #61746

estebank opened this issue Jun 11, 2019 · 10 comments · Fixed by #61757
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

std::sync::ONCE_INIT is in effect just an alias to std::sync::Once::new(), anyone using the former can use the latter since v1.2.0. I guess we can't outright deprecate ONCE_INIT given our backwards compatibility policy, but I would like to lint on its usage and at the very least call it out in the documentation ("use Once::new() instead").

This also opens the conversation around the larger problem of what to do with the parts of the API that would get superseded by new features, like const generics, or in this case const fns.

@estebank estebank added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. rust-2-breakage-wishlist In the wishlist of breaking changes that requires rust 2.0 A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Jun 11, 2019
@tesuji
Copy link
Contributor

tesuji commented Jun 11, 2019

Can we just deprecate it?

@Centril
Copy link
Contributor

Centril commented Jun 11, 2019

std::sync::ONCE_INIT is in effect just an alias to std::sync::Once::new(), anyone using the former can use the latter since v1.2.0.

Why is sync::ONCE_INIT a problem?

@euclio
Copy link
Contributor

euclio commented Jun 11, 2019

Similarly, the initializers for the Atomic family of structs were deprecated in 1.34 and have a rustfix-able suggestion to use the new() function. For example, see ATOMIC_BOOL_INIT.

@estebank
Copy link
Contributor Author

Why is sync::ONCE_INIT a problem?

"There should be only one way of doing it"™️ 🙂

I saw someone today get quite confused today as to why both existed in the first place, which required an explanation of rustc's backwards compatibility policy, const fn behavior, and rustc release schedules. I feel that we should have an aggressive funnel towards "the right way of doing things"™️ across the board, that includes linting against usages that are only kept because of historical reasons. At the very least I'd like to see the documentation updated so that it preempts the questions I got asked today 😁

@Centril
Copy link
Contributor

Centril commented Jun 11, 2019

"There should be only one way of doing it"™️ 🙂

/me Prepares plans to deprecate while true { ... } :)

In all seriousness, imo someone's confusion as to why both existed and "only one way to do it" is a rather weak justification for churn when the old way isn't harmful in any way.

At the very least I'd like to see the documentation updated so that it preempts the questions I got asked today 😁

Seems fine.

@sfackler sfackler removed the rust-2-breakage-wishlist In the wishlist of breaking changes that requires rust 2.0 label Jun 11, 2019
@sfackler
Copy link
Member

We deprecate things like ONCE_INIT 3 releases after their replacements (in this case Once::new specifically being a const fn) were added. I can't find in the release notes when that happened, but if someone tracks it down, it should be an easy PR!

@estebank
Copy link
Contributor Author

estebank commented Jun 12, 2019

@sfackler Once::new was stabilized all the way back in 1.2. Didn't find any reference to it in the release notes.

@estebank
Copy link
Contributor Author

/me Prepares plans to deprecate while true { ... } :)

warning: denote infinite loops with `loop { ... }`
 --> src/main.rs:2:5
  |
2 |     while true {  } 
  |     ^^^^^^^^^^ help: use `loop`
  |
  = note: #[warn(while_true)] on by default

😬

@sfackler
Copy link
Member

Was it callable as a const fn in 1.2.0?

@sfackler
Copy link
Member

The const-ness was made stable in the same PR as for AtomicUsize::new: https://github.com/rust-lang/rust/pull/46287/files#diff-80754b8db8699947d7b2a43a9cc17dedL159

sfackler added a commit to sfackler/rust that referenced this issue Jun 12, 2019
Once::new() has been a stable const fn for a while now.

Closes rust-lang#61746
Centril added a commit to Centril/rust that referenced this issue Jun 12, 2019
…xcrichton

Deprecate ONCE_INIT

Once::new() has been a stable const fn for a while now.

Closes rust-lang#61746
sfackler added a commit to sfackler/rust that referenced this issue Jun 13, 2019
Once::new() has been a stable const fn for a while now.

Closes rust-lang#61746
Centril added a commit to Centril/rust that referenced this issue Jun 13, 2019
…xcrichton

Deprecate ONCE_INIT in future 1.38 release

Once::new() has been a stable const fn for a while now.

Closes rust-lang#61746
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. 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.

5 participants