Enforce even more library clippy lints in CI

[ChrisDenton]

View all comments

Once again, this is best reviewed commit-by-commit since each commit fixes a lint.

I've bundled together a large(ish) number of lints because I feel they shouldn't be controversial and don't touch too much code. A large chunk of them is just telling clippy that, for example, implementing is_digit by calling is_digit just isn't going to work. I have reviewed each and every fix myself but please do double or triple check my working!

[rustbot]

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

[ChrisDenton]

Btw, they'll likely be platform specific fixes that CI tells me about so I'll probably be updating these in a few hours.

[matthiaskrgr]

@bors try @rust-timer queue

[nia-e]

mostly looks good, with a couple worries:

• clippy wants us to do a lot more method calls. this is fine but needs a perf run for sure, and i'm a smidge worried about binary size and/or bootstrap time, esp since it might put more pressure on the inliner
• a lot of these allows should be expects with a comment... but also, i'm not gonna do that to you chris ^^ that can be a later pr

let's see what a perf run comes up with before anything.

@bors try @rust-timer queue

View changes since this review

[nia-e]

huh, how did the comment from matthias not show up? apologies ^^ github has been odd

[rust-bors]

☀️ Try build successful (CI)
Build commit: 8906bf7 (8906bf76a8be8117da1f6c656fd34abfa6ebb3a0)
Base parent: e71c0f1 (e71c0f1e3395b10a8c331317be1a5c107bdf7b2e)

[clarfonthey]

Just comparing to my personal clippy lint list, what I see missing is:

• borrow_as_ptr
• default_trait_access
• doc_markdown
• filter_map_next
• if_not_else
• ignored_unit_patterns
• inconsistent_struct_constructor
• manual_assert
• manual_is_power_of_two
• map_unwrap_or
• match_same_arms
• missing_docs_in_private_items (the spiciest one, imho)
• `needless_continue
• needless_raw_string_hashes
• redundant_closure_for_method_calls
• redundant_else
• ref_as_ptr
• semicolon_if_nothing_returned
• unnecessary_semicolon

Besides missing_docs_in_private_items being almost certainly undesired at the moment (unless we want more untriaged comments), I haven't actually checked which of these are most reasonable for libstd.

[rust-timer]

Finished benchmarking commit (8906bf7): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.4%	[0.4%, 0.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 0.9%, secondary 0.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.3%	[2.1%, 2.4%]	2
Regressions ❌ (secondary)	3.3%	[3.3%, 3.3%]	1
Improvements ✅ (primary)	-1.9%	[-1.9%, -1.9%]	1
Improvements ✅ (secondary)	-2.2%	[-2.2%, -2.2%]	1
All ❌✅ (primary)	0.9%	[-1.9%, 2.4%]	3

Cycles

This perf run didn't have relevant results for this metric.

Binary size

Results (primary -0.1%, secondary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.0%, 0.5%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.1%	[-0.2%, -0.0%]	21
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.0%]	80
All ❌✅ (primary)	-0.1%	[-0.2%, 0.5%]	23

Bootstrap: 456.919s -> 457.665s (0.16%)
Artifact size: 398.99 MiB -> 398.96 MiB (-0.01%)

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[GTimothy]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Oops, sorry for that. Got inspired by reviewing you changes here :)

[ChrisDenton]

It's totally fine! The change is minor and as the message says, rebasing is a normal part of managing PRs,

[nia-e]

yep, think this addresses everything. if any of these prove problematic, we can always axe them /shrug

@bors r+ rollup=iffy

[rust-bors]

📌 Commit d712c4e has been approved by nia-e

It is now in the queue for this repository.