suggest once_cell::Lazy for non-const statics
#100507
Conversation
rustbot
added
the
T-compiler
|
r? @estebank (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| @@ -331,6 +332,10 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> { | |||
| ccx.const_kind(), | |||
| )); | |||
|
|
|||
| if let ConstContext::Static(_) = ccx.const_kind() { | |||
| err.note("consider using the `once_cell` crate: https://crates.io/crates/once_cell"); | |||
There was a problem hiding this comment.
I wonder if we could make this a structured suggestion, that suggests wrapping the const in once_cell::sync::Lazy::new(|| ... )
There was a problem hiding this comment.
Maybe not worth it, but OTOH the current note is kinda vague -- like what am I supposed to do w/ the once_cell crate exactly?
There was a problem hiding this comment.
hmm, tbh I think even mentioning the crate name is enough - it points people to read the documentation and from there they can figure out what they want (sync::OnceCell or sync::Lazy). For context, the reason I opened the issue was someone who had never heard of once_cell or lazy_static, so just knowing the crate existed would have been enough to get them unstuck.
That said, Lazy::new seems fine and pretty close to the original intent.
cameron1024
force-pushed
the
suggest-lazy
branch
from
cf4c66f
to
34e0d9a
Compare
jyn514
changed the title
jyn514
changed the title
once_cell::Lazy for non-const statics
that's a pretty big jump in meaning, and seems kind of annoying to catch all cases - I think it's fine to let it go, people who want atomics probably know to search for them already. certainly it doesn't need to be in this PR. |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
…er-errors suggest `once_cell::Lazy` for non-const statics Addresses rust-lang#100410 Some questions: - removing the `if` seems to include too many cases (e.g. calls to non-const functions inside a `const fn`), but this code excludes the following case: ```rust const FOO: Foo = non_const_fn(); ``` Should we suggest `once_cell` in this case as well? - The original issue mentions suggesting `AtomicI32` instead of `Mutex<i32>`, should this PR address that as well?
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#100186 (Mention `as_mut` alongside `as_ref` in borrowck error message) - rust-lang#100383 (Mitigate stale data reads on SGX platform) - rust-lang#100507 (suggest `once_cell::Lazy` for non-const statics) - rust-lang#100617 (Suggest the right help message for as_ref) - rust-lang#100667 (Migrate "invalid variable declaration" errors to SessionDiagnostic) - rust-lang#100709 (Migrate typeck's `used` expected symbol diagnostic to `SessionDiagnostic`) - rust-lang#100723 (Add the diagnostic translation lints to crates that don't emit them) - rust-lang#100729 (Avoid zeroing a 1kb stack buffer on every call to `std::sys::windows::fill_utf16_buf`) - rust-lang#100750 (improved diagnostic for function defined with `def`, `fun`, `func`, or `function` instead of `fn`) - rust-lang#100763 (triagebot: Autolabel `A-rustdoc-json`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Addresses #100410
Some questions:
ifseems to include too many cases (e.g. calls to non-const functions inside aconst fn), but this code excludes the following case:Should we suggest
once_cellin this case as well?AtomicI32instead ofMutex<i32>, should this PR address that as well?