Skip to content

Conversation

Philippe-Cholet
Copy link
Member

@Philippe-Cholet Philippe-Cholet commented May 8, 2024

Type of i defaults to i32 when usize should be used.
It's only a problem if this function is called with two iterators longer than i32::MAX. It seems unlikely to me but who knows?
i32 should not be the default integer type in the library. usize would have more sense so I add the lint as warning (for the library only, it's okay in tests and benchmarks).

See lint doc: https://rust-lang.github.io/rust-clippy/master/index.html#/default_numeric_fallback

@Philippe-Cholet Philippe-Cholet added this to the next milestone May 8, 2024
Copy link

codecov bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.53%. Comparing base (6814180) to head (fa94ba1).
Report is 79 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@ ## master #932 +/- ## ========================================== + Coverage 94.38% 94.53% +0.14%  ========================================== Files 48 48 Lines 6665 6947 +282 ========================================== + Hits 6291 6567 +276  - Misses 374 380 +6 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Philippe-Cholet Philippe-Cholet requested a review from phimuemue May 13, 2024 13:02
Type of `i` defaults to `i32` when `usize` should be used. It's only a problem if this function is called with two iterators longer than `i32::MAX`. It seems unlikely to me but who knows? `i32` should not be the default integer type in the library. `usize` would have more sense so I add the lint as warning (for the library only, it's okay in tests and benchmarks).
Copy link
Member

@jswrenn jswrenn left a comment

Choose a reason for hiding this comment

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

Good catch.

@jswrenn jswrenn added this pull request to the merge queue May 14, 2024
Merged via the queue into rust-itertools:master with commit 2ad9e07 May 14, 2024
@Philippe-Cholet Philippe-Cholet deleted the clippy/default_numeric_fallback branch May 14, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants