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

Fix clippy::correctness in the library #119449

Merged
merged 1 commit into from
Feb 11, 2024
Merged

Conversation

Noratrieb
Copy link
Member

@Noratrieb Noratrieb marked this pull request as ready for review December 30, 2023 17:11
@rustbot
Copy link
Collaborator

rustbot commented Dec 30, 2023

r? @joshtriplett

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 30, 2023
@Noratrieb
Copy link
Member Author

Hello Libs team! We want to enable clippy::correctness in CI because this catches bugs (and would have caught at least one bug before) (rust-lang/compiler-team#709). The easiest way to enforce is is to just pass the flag in CI, which also affects the library. This requires a few fixes seen here and of course the commitment of the libs team to continue working with it. Given that correctness has a very low false positive rate, this seems unlikely to cause problems (of course there are some funny false positives, like suggesting using is_nan in the implementation of is_nan).
Is this okay for you?

@Noratrieb Noratrieb added the I-libs-nominated Nominated for discussion during a libs team meeting. label Jan 11, 2024
@Amanieu
Copy link
Member

Amanieu commented Jan 17, 2024

We discussed this in the libs meeting and we have no objections to enabling clippy lints for the standard library. We can always revert later if this turns out to cause issues.

@Amanieu Amanieu removed the I-libs-nominated Nominated for discussion during a libs team meeting. label Jan 17, 2024
@Noratrieb
Copy link
Member Author

Sounds good! Would appreciate a review :)

@@ -211,6 +211,8 @@
#![cfg_attr(not(target_has_atomic_load_store = "8"), allow(dead_code))]
#![cfg_attr(not(target_has_atomic_load_store = "8"), allow(unused_imports))]
#![rustc_diagnostic_item = "atomic_mod"]
// AtomicPtr intrinsics are unsafe and take pointers, but do not dereference them.
#![allow(clippy::not_unsafe_ptr_arg_deref)]
Copy link
Member

Choose a reason for hiding this comment

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

Don't these call intrinsics which are themselves unsafe functions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, they are unsafe, which is what clippy is concerned about. I elaborated the comment to explain what exactly is happening.

@joshtriplett
Copy link
Member

r? libs

@rustbot rustbot assigned cuviper and unassigned joshtriplett Feb 11, 2024
@cuviper
Copy link
Member

cuviper commented Feb 11, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 11, 2024

📌 Commit e55716e has been approved by cuviper

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 11, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 11, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#117740 (Add some links and minor explanatory comments to `std::fmt`)
 - rust-lang#118307 (Remove an unneeded helper from the tuple library code)
 - rust-lang#119242 (Suggest less bug-prone construction of Duration in docs)
 - rust-lang#119449 (Fix `clippy::correctness` in the library)
 - rust-lang#120307 (core: add Duration constructors)
 - rust-lang#120459 (Document various I/O descriptor/handle conversions)
 - rust-lang#120729 (Update mdbook to 0.4.37)
 - rust-lang#120763 (Suggest pattern tests when modifying exhaustiveness)
 - rust-lang#120906 (Remove myself from some review rotations)
 - rust-lang#120916 (Subtree update of ` rust-analyzer`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 11, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#117740 (Add some links and minor explanatory comments to `std::fmt`)
 - rust-lang#118307 (Remove an unneeded helper from the tuple library code)
 - rust-lang#119242 (Suggest less bug-prone construction of Duration in docs)
 - rust-lang#119449 (Fix `clippy::correctness` in the library)
 - rust-lang#120307 (core: add Duration constructors)
 - rust-lang#120459 (Document various I/O descriptor/handle conversions)
 - rust-lang#120729 (Update mdbook to 0.4.37)
 - rust-lang#120763 (Suggest pattern tests when modifying exhaustiveness)
 - rust-lang#120906 (Remove myself from some review rotations)
 - rust-lang#120916 (Subtree update of ` rust-analyzer`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0c5d8d3 into rust-lang:master Feb 11, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 11, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 11, 2024
Rollup merge of rust-lang#119449 - Nilstrieb:library-clippy, r=cuviper

Fix `clippy::correctness` in the library

needs rust-lang/backtrace-rs#579 to be complete

for rust-lang/compiler-team#709
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants