Add trivially_copy_pass_by_ref to the lint set.
#73
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DJMcNab
reviewed
Nov 21, 2024
| // See https://linebender.org/wiki/canonical-lints/ | ||
| // These lints aren't included in Cargo.toml because they | ||
| // shouldn't apply to examples and tests | ||
| #![warn(unused_crate_dependencies)] | ||
| #![warn(clippy::print_stdout, clippy::print_stderr)] | ||
| // END LINEBENDER LINT SET |
There was a problem hiding this comment.
Previous discussion here #68 (comment)
There was a problem hiding this comment.
The rustfmt problem is valid, but looking at this in the wild it's not immediately obvious where the set ends. I'll need to think about this a bit more.
There was a problem hiding this comment.
Yeah. I think the problem comes if there are no other trailing attributes; we want this to come after crate level docs, so it's plausible that there'd be no other attributes.
Maybe the solution is for the pattern to be:
// LINEBENDER LINT SET ...
...
// END LINEBENDER LINT SET
#![cfg_attr(docsrs, feature(doc_auto_cfg))]?
There was a problem hiding this comment.
I like it, as we want that present everywhere anyway.
DJMcNab
approved these changes
Nov 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an old standard lint from the Druid days, still enabled for Druid, Piet, Kurbo, Xilem, and was just removed from Parley two days ago which triggered this PR.
I think it makes sense to keep it as part of the standard set. It helps detect potential micro-optimizations. The default Clippy configuration option
avoid-breaking-exported-apiistruewhich means it won't affect the public API.The main downside is that if we have a tiny struct which gets suggested for copy and then later that struct grows larger, we will need to manually update the methods to take a reference instead. Probably a good idea to add a code comment for potential growers about this.
The
trivial-copy-size-limitconfiguration of 16 bytes is also a continuation of an old decision. The rationale for optimizing for 64-bit is even more true today with even low-end mobile phones having 64-bit CPUs.I also bumped the linebender lint set version, but only for the
Cargo.tomlbunch. I think it can only cause wasted effort to bump the version in other files that have zero changes.