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

Clippy cleanup for all targets and nighly rust (also support 1.44.0) #10445

Merged
merged 8 commits into from Jun 9, 2020

Conversation

svenski123
Copy link
Contributor

Problem

Further clippy warning edits, this time for all targets and against rust 1.45.0 (nightly)

Summary of Changes

  • Numerous edits to address all clippy warnings raised by rustup run nightly cargo clippy --workspace --all-targets
  • Integer casts from floating point infinity (i.e. from divide by zero) result in zero in rust 1.43 but MAX_INT in 1.45 which broke the bloom filter and rent collector in some test cases; these classes are fixed however this rust language change may cause issues elsewhere (see reference links below)

References

@codecov
Copy link

codecov bot commented Jun 6, 2020

Codecov Report

Merging #10445 into master will decrease coverage by 0.0%.
The diff coverage is 75.4%.

@@            Coverage Diff            @@
##           master   #10445     +/-   ##
=========================================
- Coverage    81.6%    81.6%   -0.1%     
=========================================
  Files         296      296             
  Lines       69340    69320     -20     
=========================================
- Hits        56646    56620     -26     
- Misses      12694    12700      +6     

@ryoqun ryoqun self-assigned this Jun 8, 2020
@ryoqun ryoqun self-requested a review June 8, 2020 15:32
ryoqun
ryoqun previously requested changes Jun 8, 2020
Copy link
Member

@ryoqun ryoqun left a comment

Choose a reason for hiding this comment

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

@svenski123 Thank you so much for working on these cleaning ups yet more again!!

So far, very good! Could you check my some comments and rebase to fix the conflicts?

runtime/src/accounts_db.rs Outdated Show resolved Hide resolved
runtime/src/accounts_db.rs Show resolved Hide resolved
cli/src/cli.rs Show resolved Hide resolved
@ryoqun ryoqun changed the title Clippy cleanup for all targets and nighly rust Clippy cleanup for all targets and nighly rust (also support 1.44.0) Jun 8, 2020
@mergify mergify bot dismissed ryoqun’s stale review June 8, 2020 19:11

Pull request has been modified.

svenski123 and others added 8 commits June 8, 2020 21:33
minor refactoring in:
- cli/src/cli.rs
- cli/src/offline/blockhash_query.rs
- logger/src/lib.rs
- runtime/src/accounts_db.rs

expect some performance improvement AccountsDB::clean_accounts()
replace ref-to-arc with ref parameters where arc not cloned
@@ -245,7 +248,8 @@ mod tests {
let key = Keypair::new();
let index = AccountsIndex::<bool>::default();
let ancestors = HashMap::new();
assert!(index.get(&key.pubkey(), &ancestors).is_none());
assert!(index.get(&key.pubkey(), Some(&ancestors)).is_none());
assert!(index.get(&key.pubkey(), None).is_none());
Copy link
Member

Choose a reason for hiding this comment

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

Nice for adding another case for your change. :)

Copy link
Member

@ryoqun ryoqun left a comment

Choose a reason for hiding this comment

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

LGTM!!! Thanks for working on this!

@ryoqun ryoqun merged commit e23340d into solana-labs:master Jun 9, 2020
@ryoqun
Copy link
Member

ryoqun commented Jun 16, 2020

This pr was the basis for #10585 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants