From 8ec05a3e90b349834337e2f6e58b315cc8244b7e Mon Sep 17 00:00:00 2001 From: Kaur Kuut Date: Thu, 21 Nov 2024 19:12:31 +0200 Subject: [PATCH] Add `trivially_copy_pass_by_ref` to the lint set. (#73) This is an old standard lint from the Druid days, still enabled for [Druid](https://github.com/linebender/druid/blob/b831b5fe1597d5ec1fc3379cc1b39f3dd106e220/druid/src/lib.rs#L135), [Piet](https://github.com/linebender/piet/blob/aef3254b39c8aa291e3fb2d53fd941acd2f04be9/piet/src/lib.rs#L28), [Kurbo](https://github.com/linebender/kurbo/blob/1313f16b29fbbc84fc29fa4924c5dc9560460e83/src/lib.rs#L70), [Xilem](https://github.com/linebender/xilem/blob/10dc9d171ce08bf207ec3057b47d1405606faf58/xilem/src/lib.rs#L5), and was just [removed from Parley two days ago](https://github.com/linebender/parley/pull/180) 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-api` is `true`](https://doc.rust-lang.org/clippy/lint_configuration.html#avoid-breaking-exported-api) which 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-limit` configuration 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.toml` bunch. I think it can only cause wasted effort to bump the version in other files that have zero changes. --- content/wiki/canonical_lints.md | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/content/wiki/canonical_lints.md b/content/wiki/canonical_lints.md index e565d0d..2ec4c84 100644 --- a/content/wiki/canonical_lints.md +++ b/content/wiki/canonical_lints.md @@ -2,14 +2,16 @@ title = "Canonical lint set for Linebender projects" +++ -All Linebender projects should include the following set of lints in their `Cargo.toml`: +All Linebender projects should include the following set of lints: + +## `Cargo.toml` ```toml [lints] # This one may vary depending on the project. rust.unsafe_code = "forbid" -# LINEBENDER LINT SET - v1 +# LINEBENDER LINT SET - Cargo.toml - v2 # See https://linebender.org/wiki/canonical-lints/ rust.keyword_idents_2024 = "forbid" rust.non_ascii_idents = "forbid" @@ -57,6 +59,7 @@ clippy.semicolon_if_nothing_returned = "warn" clippy.shadow_unrelated = "warn" clippy.should_panic_without_expect = "warn" clippy.todo = "warn" +clippy.trivially_copy_pass_by_ref = "warn" clippy.unseparated_literal_suffix = "warn" clippy.use_self = "warn" clippy.wildcard_imports = "warn" @@ -68,15 +71,32 @@ clippy.wildcard_dependencies = "warn" # END LINEBENDER LINT SET ``` -And in their `lib.rs`: +## `lib.rs` ```rust -// LINEBENDER LINT SET - v1 +// LINEBENDER LINT SET - lib.rs - v1 // 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 +#![cfg_attr(docsrs, feature(doc_auto_cfg))] +``` + +## `.clippy.toml` + +```toml +# LINEBENDER LINT SET - .clippy.toml - v1 +# See https://linebender.org/wiki/canonical-lints/ + +# The default Clippy value is capped at 8 bytes, which was chosen to improve performance on 32-bit. +# Given that we are building for the future and even low-end mobile phones have 64-bit CPUs, +# it makes sense to optimize for 64-bit and accept the performance hits on 32-bit. +# 16 bytes is the number of bytes that fits into two 64-bit CPU registers. +trivial-copy-size-limit = 16 + +# END LINEBENDER LINT SET ``` This is a curated list: Clippy has a *lot* of lints, and most of them are not included above.