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

lints: add #![forbid] lints per crate #375

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hinto-janai
Copy link
Contributor

@hinto-janai hinto-janai commented Jan 26, 2025

What

  • Adds more specific #![forbid] lints for all crates where applicable
  • Adds unsafe_code = "deny" to Cargo.toml

Why

Allows us to uphold certain static invariants within crates, the most useful one being unsafe_code.

Within the crates that use unsafe, only a small portion of the crate needs unsafe. Notating the files that need it with #[expect(unsafe_code)] allows the rest of the crate to be known safe (at least the keyword, obviously using unsafe incorrectly can cause safe code to be unsafe).

I.e. it defines and restricts the unsafe interface to certain portions of the crate.

@github-actions github-actions bot added A-p2p Related to P2P. A-consensus Related to consensus. A-cryptonight Related to Cryptonight. A-storage Related to storage. A-helper Related to cuprate-helper. A-net Related to networking. A-pruning Related to pruning. A-types Related to types. A-rpc Related to RPC. A-zmq Related to ZMQ. A-binaries Related to binaries. A-constants Related to constants. labels Jan 26, 2025
@github-actions github-actions bot added A-dependency Related to dependencies, or changes to a Cargo.{toml,lock} file. A-workspace Changes to a root workspace file or general repo file. labels Jan 26, 2025
Comment on lines +2 to +15
#![forbid(
clippy::missing_assert_message,
clippy::missing_docs_in_private_items,
clippy::missing_errors_doc,
clippy::missing_panics_doc,
clippy::should_panic_without_expect,
clippy::single_char_lifetime_names,
missing_docs,
unsafe_code,
unused_results,
missing_copy_implementations,
missing_debug_implementations,
reason = "Crate-specific lints. There should be good reasoning when removing these."
)]
Copy link
Contributor Author

@hinto-janai hinto-janai Jan 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the full list of lints, all other crates have some subset of this. Only a few allow unsafe_code, I will list them:

@@ -69,6 +69,7 @@ impl_thread_percent! {
///
/// On macOS and *BSD: +20
/// On Linux: +19
#[expect(unsafe_code, reason = "Must call C")]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cuprate_helper uses unsafe 1 time here.

@@ -1,5 +1,7 @@
//! Wrapper type for partially-`unsafe` usage of `T: !Send`.

#![expect(unsafe_code)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cuprate_blockchain uses unsafe for UnsafeSendable.

clippy::significant_drop_tightening
clippy::significant_drop_tightening,
)]
#![forbid(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cuprate_database uses unsafe for:

  • pub unsafe trait DatabaseRo
  • heed

The #[expect]s everywhere may seem a bit messy although it does mean the rest of cuprate_database is known not to use unsafe, this may be worth it.

@hinto-janai hinto-janai added the P-low Low priority. label Jan 26, 2025
@hinto-janai hinto-janai marked this pull request as ready for review January 26, 2025 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-binaries Related to binaries. A-consensus Related to consensus. A-constants Related to constants. A-cryptonight Related to Cryptonight. A-dependency Related to dependencies, or changes to a Cargo.{toml,lock} file. A-helper Related to cuprate-helper. A-net Related to networking. A-p2p Related to P2P. A-pruning Related to pruning. A-rpc Related to RPC. A-storage Related to storage. A-types Related to types. A-workspace Changes to a root workspace file or general repo file. A-zmq Related to ZMQ. P-low Low priority.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants