-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Make trivial-copy-size-limit
consistently the size of the target pointer
#13319
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
Conversation
0d48fc4
to
e966c5e
Compare
The original justification for limiting it to eight on 64-bit targets was quite flimsy. It makes It would be nice if the lint could be target independent, but struct sizes already aren't. |
It means it's not small enough to lint, not that it's too big to pass by value. That's a separate config |
Sorry I had lint/not lint backwards (was half thinking of |
☔ The latest upstream changes (presumably #13286) made this pull request unmergeable. Please resolve the merge conflicts. |
This comment has been minimized.
This comment has been minimized.
e966c5e
to
849ed1a
Compare
trivial-copy-size-limit
target independenttrivial-copy-size-limit
consistently the size of the target pointer
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 |
849ed1a
to
ae0e3b7
Compare
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
trivial-copy-size-limit
Since practically speaking it's almost always 8, let's go with that as the unconditional default to make it easier to understandNow 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)