Add stacks-common to clippy CI and fix errors #5598
Conversation
Signed-off-by: Jacinta Ferrant <[email protected]>
… into feat/clippy-ci-stacks-common
Signed-off-by: Jacinta Ferrant <[email protected]>
Signed-off-by: Jacinta Ferrant <[email protected]>
… into feat/clippy-ci-stacks-common
… into feat/clippy-ci-stacks-common
| assert!(CoinbaseInterval::check_order(&emissions_schedule)); | ||
| emissions_schedule | ||
| }); | ||
| // This static value is lazily initialized using `OnceLock` to avoid unnecessary |
There was a problem hiding this comment.
What was the issue with LazyCell here? Using LazyCell (or LazyLock if you need thread safety) is more concise than OnceLock
There was a problem hiding this comment.
Yes. If we used lazycell it caused errors related to mutability. I added the comment to better explain.
There was a problem hiding this comment.
I don't mind using lazyLock but once lock is the recommended over lazycell and lazylock. I can be persuaded otherwise but its the recommended standard.
There was a problem hiding this comment.
You have the same interior mutability issues with OnceLock as LazyCell, as both provide a get_mut() function.
The reason the warning went away is that you changed COINBASE_INTERVALS_MAINNET from const to static, so now it doesn't trigger Clippy's declare_interior_mutable_const because there's no const
I'd stick with the LazyCell, it's cleaner and more efficient (not thread-safe, so no spinlock or mutex required to access it). Just make it static to kill the warning
There was a problem hiding this comment.
I cannot make it static.
`UnsafeCell<cell::lazy::State<[CoinbaseInterval; 5], fn() -> [CoinbaseInterval; 5]>>` cannot be shared between threads safely
within `LazyCell<[CoinbaseInterval; 5]>`, the trait `Sync` is not implemented for `UnsafeCell<cell::lazy::State<[CoinbaseInterval; 5], fn() -> [CoinbaseInterval; 5]>>`, which is required by `LazyCell<[CoinbaseInterval; 5]>: Sync`
shared static variables must have a type that implements `SyncI could just silence the warning if you prefer? But I cannot just declare it static and still use LazyCell
There was a problem hiding this comment.
That error means it's shared by multiple threads and must use a Sync type, like LazyLock. OnceLock works also, it's just cumbersome and awkward to work with. We use LazyLock elsewhere in the code, it's basically the modern stdlib replacement for lazy_static!. See also PR #5055
Signed-off-by: Jacinta Ferrant <[email protected]>
There were actually a bunch of compile errors (not just clippy errors) for this library if all-features flag was set. Tried my best to fix them. I verified that all the unit tests for stacks-common still pass.