-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Apply clippy suggestions for rustc and core #89709
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
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 |
---|---|---|
|
@@ -14,7 +14,7 @@ const BASE_64: &[u8; MAX_BASE as usize] = | |
|
||
#[inline] | ||
pub fn push_str(mut n: u128, base: usize, output: &mut String) { | ||
debug_assert!(base >= 2 && base <= MAX_BASE); | ||
debug_assert!((2..=MAX_BASE).contains(&base)); | ||
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. It would be nice if we could use 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.
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. Compiler code can use unstable library features. 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. @clemenswasser If this PR is re-landed (after the #89905 revert), it would be good to fix this. |
||
let mut s = [0u8; 128]; | ||
let mut index = 0; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -390,7 +390,7 @@ impl<O: ForestObligation> ObligationForest<O> { | |
.map(|(index, _node)| Error { error: error.clone(), backtrace: self.error_at(index) }) | ||
.collect(); | ||
|
||
self.compress(|_| assert!(false)); | ||
self.compress(|_| unreachable!()); | ||
errors | ||
} | ||
|
||
|
@@ -612,7 +612,7 @@ impl<O: ForestObligation> ObligationForest<O> { | |
fn compress(&mut self, mut outcome_cb: impl FnMut(&O)) { | ||
let orig_nodes_len = self.nodes.len(); | ||
let mut node_rewrites: Vec<_> = std::mem::take(&mut self.reused_node_vec); | ||
debug_assert!(node_rewrites.is_empty()); | ||
assert!(node_rewrites.is_empty()); | ||
node_rewrites.extend(0..orig_nodes_len); | ||
let mut dead_nodes = 0; | ||
|
||
|
@@ -623,13 +623,13 @@ impl<O: ForestObligation> ObligationForest<O> { | |
// self.nodes[0..index - dead_nodes] are the first remaining nodes | ||
// self.nodes[index - dead_nodes..index] are all dead | ||
// self.nodes[index..] are unchanged | ||
for index in 0..orig_nodes_len { | ||
for (index, node_rewrite) in node_rewrites.iter_mut().enumerate() { | ||
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. Is it possible |
||
let node = &self.nodes[index]; | ||
match node.state.get() { | ||
NodeState::Pending | NodeState::Waiting => { | ||
if dead_nodes > 0 { | ||
self.nodes.swap(index, index - dead_nodes); | ||
node_rewrites[index] -= dead_nodes; | ||
*node_rewrite -= dead_nodes; | ||
} | ||
} | ||
NodeState::Done => { | ||
|
@@ -646,7 +646,7 @@ impl<O: ForestObligation> ObligationForest<O> { | |
} | ||
// Extract the success stories. | ||
outcome_cb(&node.obligation); | ||
node_rewrites[index] = orig_nodes_len; | ||
*node_rewrite = orig_nodes_len; | ||
dead_nodes += 1; | ||
} | ||
NodeState::Error => { | ||
|
@@ -655,7 +655,7 @@ impl<O: ForestObligation> ObligationForest<O> { | |
// check against. | ||
self.active_cache.remove(&node.obligation.as_cache_key()); | ||
self.insert_into_error_cache(index); | ||
node_rewrites[index] = orig_nodes_len; | ||
*node_rewrite = orig_nodes_len; | ||
dead_nodes += 1; | ||
} | ||
NodeState::Success => unreachable!(), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -257,11 +257,7 @@ impl<K: Eq + Hash, V> SsoHashMap<K, V> { | |
pub fn remove(&mut self, key: &K) -> Option<V> { | ||
match self { | ||
SsoHashMap::Array(array) => { | ||
if let Some(index) = array.iter().position(|(k, _v)| k == key) { | ||
Some(array.swap_remove(index).1) | ||
} else { | ||
None | ||
} | ||
array.iter().position(|(k, _v)| k == key).map(|index| array.swap_remove(index).1) | ||
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 know other data structures have some instances of open coding 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 would worry less about bloat and more about that instance of |
||
} | ||
SsoHashMap::Map(map) => map.remove(key), | ||
} | ||
|
@@ -272,11 +268,7 @@ impl<K: Eq + Hash, V> SsoHashMap<K, V> { | |
pub fn remove_entry(&mut self, key: &K) -> Option<(K, V)> { | ||
match self { | ||
SsoHashMap::Array(array) => { | ||
if let Some(index) = array.iter().position(|(k, _v)| k == key) { | ||
Some(array.swap_remove(index)) | ||
} else { | ||
None | ||
} | ||
array.iter().position(|(k, _v)| k == key).map(|index| array.swap_remove(index)) | ||
} | ||
SsoHashMap::Map(map) => map.remove_entry(key), | ||
} | ||
|
@@ -423,14 +415,14 @@ impl<K, V> IntoIterator for SsoHashMap<K, V> { | |
|
||
/// adapts Item of array reference iterator to Item of hashmap reference iterator. | ||
#[inline(always)] | ||
fn adapt_array_ref_it<K, V>(pair: &'a (K, V)) -> (&'a K, &'a V) { | ||
fn adapt_array_ref_it<K, V>(pair: &(K, V)) -> (&K, &V) { | ||
let (a, b) = pair; | ||
(a, b) | ||
} | ||
|
||
/// adapts Item of array mut reference iterator to Item of hashmap mut reference iterator. | ||
#[inline(always)] | ||
fn adapt_array_mut_it<K, V>(pair: &'a mut (K, V)) -> (&'a K, &'a mut V) { | ||
fn adapt_array_mut_it<K, V>(pair: &mut (K, V)) -> (&K, &mut V) { | ||
let (a, b) = pair; | ||
(a, b) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -990,9 +990,8 @@ impl<R: Idx, C: Idx> BitMatrix<R, C> { | |
pub fn insert_all_into_row(&mut self, row: R) { | ||
assert!(row.index() < self.num_rows); | ||
let (start, end) = self.range(row); | ||
let words = &mut self.words[..]; | ||
for index in start..end { | ||
words[index] = !0; | ||
for word in self.words[start..end].iter_mut() { | ||
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 didn't get to this in time, but |
||
*word = !0; | ||
} | ||
Comment on lines
-993
to
995
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. This entire thing can just be |
||
self.clear_excess_bits(row); | ||
} | ||
|
@@ -1144,7 +1143,7 @@ impl<R: Idx, C: Idx> SparseBitMatrix<R, C> { | |
|
||
/// Iterates through all the columns set to true in a given row of | ||
/// the matrix. | ||
pub fn iter<'a>(&'a self, row: R) -> impl Iterator<Item = C> + 'a { | ||
pub fn iter(&self, row: R) -> impl Iterator<Item = C> + '_ { | ||
self.row(row).into_iter().flat_map(|r| r.iter()) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -349,14 +349,14 @@ impl<'a> SessionDiagnosticDeriveBuilder<'a> { | |
) -> Result<proc_macro2::TokenStream, SessionDiagnosticDeriveError> { | ||
let field_binding = &info.binding.binding; | ||
|
||
let option_ty = option_inner_ty(&info.ty); | ||
let option_ty = option_inner_ty(info.ty); | ||
|
||
let generated_code = self.generate_non_option_field_code( | ||
attr, | ||
FieldInfo { | ||
vis: info.vis, | ||
binding: info.binding, | ||
ty: option_ty.unwrap_or(&info.ty), | ||
ty: option_ty.unwrap_or(info.ty), | ||
span: info.span, | ||
}, | ||
)?; | ||
|
@@ -388,7 +388,7 @@ impl<'a> SessionDiagnosticDeriveBuilder<'a> { | |
let formatted_str = self.build_format(&s.value(), attr.span()); | ||
match name { | ||
"message" => { | ||
if type_matches_path(&info.ty, &["rustc_span", "Span"]) { | ||
if type_matches_path(info.ty, &["rustc_span", "Span"]) { | ||
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. Nice, removing redundant 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. Is there a simple way I can 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. In theory we could simply deny the lint via x.py clippy, but in reality it seems that diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs
index 28e7f1fdca7..cf21a4dc857 100644
--- a/src/bootstrap/check.rs
+++ b/src/bootstrap/check.rs
@@ -44,6 +44,7 @@ fn strings<'a>(arr: &'a [&str]) -> impl Iterator<Item = String> + 'a {
}
args.extend(strings(&["--", "--cap-lints", "warn"]));
args.extend(ignored_lints.iter().map(|lint| format!("-Aclippy::{}", lint)));
+ args.push("-Dclippy::needless_borrow".into());
args
} else {
vec![] |
||
quote! { | ||
#diag.set_span(*#field_binding); | ||
#diag.set_primary_message(#formatted_str); | ||
|
@@ -401,7 +401,7 @@ impl<'a> SessionDiagnosticDeriveBuilder<'a> { | |
} | ||
} | ||
"label" => { | ||
if type_matches_path(&info.ty, &["rustc_span", "Span"]) { | ||
if type_matches_path(info.ty, &["rustc_span", "Span"]) { | ||
quote! { | ||
#diag.span_label(*#field_binding, #formatted_str); | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.