WIP: Lots of fixes from clippy #46960
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
clarfonthey
force-pushed
the
fixes_from_clippy
branch
from
821c187 to
51d91e0
Compare
|
I think that changes to the Vec and similar parts should be done carefully, with benchmarks too. |
|
This is awesome! The |
| @@ -27,7 +27,7 @@ | |||
| //! We can explicitly specify `hello_world`'s lifetime as well: | |||
| //! | |||
| //! ``` | |||
| //! let hello_world: &'static str = "Hello, world!"; | |||
There was a problem hiding this comment.
this is in documentation showing how to explicitly specify a lifetime, so it shouldn't be removed
kennytm
added
the
S-waiting-on-author
| @@ -105,6 +105,7 @@ def error(f, e, z): | |||
|
|
|||
| //! Tables of approximations of powers of ten. | |||
| //! DO NOT MODIFY: Generated by `src/etc/dec2flt_table.py` | |||
| #![cfg_attr(feature = "cargo-clippy", allow(unreadable_literal))] | |||
There was a problem hiding this comment.
Can annotations like this be omitted? We don't run clippy in the main repository regularly so I'd prefer to not land annotations like this which are basically unused by all our infrastructure.
| @@ -0,0 +1 @@ | |||
| doc-valid-idents = ["TimSort"] | |||
There was a problem hiding this comment.
Similar to above, can this be omitted?
| @@ -27,7 +27,7 @@ | |||
| //! We can explicitly specify `hello_world`'s lifetime as well: | |||
| //! | |||
| //! ``` | |||
| //! let hello_world: &'static str = "Hello, world!"; | |||
| @@ -2337,22 +2338,20 @@ impl<T> Iterator for IntoIter<T> { | |||
| unsafe { | |||
| if self.ptr as *const _ == self.end { | |||
| None | |||
| } else if mem::size_of::<T>() == 0 { | |||
There was a problem hiding this comment.
Can this be left as it was, it's a bit clearer in terms of control flow previously
There was a problem hiding this comment.
I can revert it but I'm a little confused how else { if is more clear than else if; it just adds unnecessary indentation IMHO.
| if mem::size_of::<T>() == 0 { | ||
| // See above for why 'ptr.offset' isn't used | ||
| self.end = arith_offset(self.end as *const i8, -1) as *mut T; | ||
| } else if mem::size_of::<T>() == 0 { |
There was a problem hiding this comment.
Similar to above, can this be left as it was?
| @@ -2485,7 +2482,7 @@ impl<'a, T> DoubleEndedIterator for Drain<'a, T> { | |||
| impl<'a, T> Drop for Drain<'a, T> { | |||
| fn drop(&mut self) { | |||
| // exhaust self first | |||
| while let Some(_) = self.next() {} | |||
| for _ in self.by_ref() {} | |||
There was a problem hiding this comment.
Can this be left as it was? This is a pretty unsafe implementation so having it be as clear/simple as possible (less layers of abstraction) makes it easier to audit and see what's going on.
| } else { | ||
| if (tail - head) >= cmp::min((cap - tail), head) { |
There was a problem hiding this comment.
Similar to aove, can this be left as it was?
| // NOTE: The following code was generated by "src/etc/char_private.py", | ||
| // do not edit directly! | ||
| //! NOTE: The following code was generated by `src/etc/char_private.py`; | ||
| //! do not edit directly! |
There was a problem hiding this comment.
This I believe wasn't intended to be a doc comment, can the ! be removed.
|
I think it'll be best to just resubmit this PR as a series of smaller PRs for clarity. Closing this one. |
Will continue to add more changes to this until it's merged. This was the result of running cargo clippy on libcore and liballoc, although I didn't get close to fixing everything. I'll continue to add changes as I look through what clippy says, but I'm going to put this up now so that people can review the existing changes.
I'd be willing to split this into multiple commits if preferred.