Skip to content

Conversation

Alexendoo
Copy link
Member

@Alexendoo Alexendoo commented Aug 28, 2024

Fixes https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Ambiguous.20default.20value.20for.20.60trivial-copy-size-limit.60

The current situation is

Target width trivial-copy-size-limit
8-bit 2
16-bit 4
32-bit 8
64-bit 8

Since practically speaking it's almost always 8, let's go with that as the unconditional default to make it easier to understand

Now defaults to target_pointer_width

changelog: [trivial-copy-size-limit] now also defaults to the size of a target pointer (unchanged for 64-bit targets)

@rustbot
Copy link
Collaborator

rustbot commented Aug 28, 2024

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 28, 2024
@rust-cloud-vms rust-cloud-vms bot force-pushed the trivial-copy-size-limit branch from 0d48fc4 to e966c5e Compare August 28, 2024 18:06
@Jarcho
Copy link
Contributor

Jarcho commented Aug 28, 2024

The original justification for limiting it to eight on 64-bit targets was quite flimsy. It makes (usize, usize) ok on everything but 64-bit targets. This means something like struct S(&'static [u8]) is too big to pass by value, which is rather ridiculous.

It would be nice if the lint could be target independent, but struct sizes already aren't.

@Alexendoo
Copy link
Member Author

This means something like struct S(&'static [u8]) is too big to pass by value

It means it's not small enough to lint, not that it's too big to pass by value. That's a separate config pass-by-value-size-limit

@Jarcho
Copy link
Contributor

Jarcho commented Aug 29, 2024

Sorry I had lint/not lint backwards (was half thinking of large_types_passed_by_value). &S would be trigger trivially_copy_pass_by_ref on 32-bit targets, but not on 64-bit targets. Basically fat pointers should behave consistently.

@bors
Copy link
Contributor

bors commented Oct 1, 2024

☔ The latest upstream changes (presumably #13286) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot

This comment has been minimized.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 31, 2025
@Alexendoo Alexendoo force-pushed the trivial-copy-size-limit branch from e966c5e to 849ed1a Compare April 2, 2025 14:30
@Alexendoo Alexendoo changed the title Make trivial-copy-size-limit target independent Make trivial-copy-size-limit consistently the size of the target pointer Apr 2, 2025
@Alexendoo
Copy link
Member Author

Changed it to be the width of the target pointer, this halves the limit for smaller targets but keeps 64-bit the same

Increasing 64-bit's limit is also an option, but it led to many more cases being linted

@Alexendoo Alexendoo added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Apr 2, 2025
@Alexendoo Alexendoo force-pushed the trivial-copy-size-limit branch from 849ed1a to ae0e3b7 Compare April 3, 2025 12:18
@Jarcho Jarcho added this pull request to the merge queue May 21, 2025
Merged via the queue into rust-lang:master with commit 9fa448a May 21, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants