-
Notifications
You must be signed in to change notification settings - Fork 317
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
clippy #740
clippy #740
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -475,6 +475,8 @@ fn merge_default(c: &mut Criterion) { | |
let mut data1 = vec![0; 1024]; | ||
let mut data2 = vec![0; 800]; | ||
let mut x = 0; | ||
|
||
#[allow(clippy::explicit_counter_loop, clippy::unused_enumerate_index)] | ||
Comment on lines
+478
to
+479
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what this benchmark is about. I'm only guessing that calling enumerate and then ignoring the index is intentional. Same goes for the similar ones below. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mightyiam After merging this PR, I search for PRs/issues about clippy (and closed some), saw #618 and eventually ran There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we run clippy four times because of a matrix value What version toolchain did you get an error with and what is the error exactly, please? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh! unused_enumerate_index is added in 1.75 and I'm still on 1.74, my bad sorry! I'm so used on thinking I can't use recent features of Rust I did not thought it could be such a recent lint. |
||
for (_, elt) in data1.iter_mut().enumerate() { | ||
*elt = x; | ||
x += 1; | ||
|
@@ -501,6 +503,8 @@ fn merge_by_cmp(c: &mut Criterion) { | |
let mut data1 = vec![0; 1024]; | ||
let mut data2 = vec![0; 800]; | ||
let mut x = 0; | ||
|
||
#[allow(clippy::explicit_counter_loop, clippy::unused_enumerate_index)] | ||
for (_, elt) in data1.iter_mut().enumerate() { | ||
*elt = x; | ||
x += 1; | ||
|
@@ -527,6 +531,8 @@ fn merge_by_lt(c: &mut Criterion) { | |
let mut data1 = vec![0; 1024]; | ||
let mut data2 = vec![0; 800]; | ||
let mut x = 0; | ||
|
||
#[allow(clippy::explicit_counter_loop, clippy::unused_enumerate_index)] | ||
for (_, elt) in data1.iter_mut().enumerate() { | ||
*elt = x; | ||
x += 1; | ||
|
@@ -553,6 +559,8 @@ fn kmerge_default(c: &mut Criterion) { | |
let mut data1 = vec![0; 1024]; | ||
let mut data2 = vec![0; 800]; | ||
let mut x = 0; | ||
|
||
#[allow(clippy::explicit_counter_loop, clippy::unused_enumerate_index)] | ||
for (_, elt) in data1.iter_mut().enumerate() { | ||
*elt = x; | ||
x += 1; | ||
|
@@ -592,7 +600,7 @@ fn kmerge_tenway(c: &mut Criterion) { | |
|
||
let mut chunks = Vec::new(); | ||
let mut rest = &mut data[..]; | ||
while rest.len() > 0 { | ||
while !rest.is_empty() { | ||
Philippe-Cholet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let chunk_len = 1 + rng(&mut state) % 512; | ||
let chunk_len = cmp::min(rest.len(), chunk_len as usize); | ||
let (fst, tail) = { rest }.split_at_mut(chunk_len); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,19 +29,13 @@ impl<A, B> EitherOrBoth<A, B> { | |
/// If `Left`, return true. Otherwise, return false. | ||
/// Exclusive version of [`has_left`](EitherOrBoth::has_left). | ||
pub fn is_left(&self) -> bool { | ||
match *self { | ||
Left(_) => true, | ||
_ => false, | ||
} | ||
matches!(self, Left(_)) | ||
} | ||
|
||
/// If `Right`, return true. Otherwise, return false. | ||
/// Exclusive version of [`has_right`](EitherOrBoth::has_right). | ||
pub fn is_right(&self) -> bool { | ||
match *self { | ||
Right(_) => true, | ||
_ => false, | ||
} | ||
matches!(self, Right(_)) | ||
} | ||
|
||
/// If `Both`, return true. Otherwise, return false. | ||
|
@@ -500,6 +494,7 @@ impl<T> EitherOrBoth<T, T> { | |
} | ||
} | ||
|
||
#[allow(clippy::from_over_into)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does anyone know why we're implementing Into instead of From? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a point of not doing it so I suggest you convert it to a EDIT: After searching, it originated in #327 and nobody said it should not be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's an issue for that. Now that shouldn't block this PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I sure want this PR to be merged as soon as possible as you made quite a work here! |
||
impl<A, B> Into<Option<Either<A, B>>> for EitherOrBoth<A, B> { | ||
fn into(self) -> Option<Either<A, B>> { | ||
match self { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this here. Good enough?