Skip to content

Commit

Permalink
Add trivially_copy_pass_by_ref to the lint set. (#73)
Browse files Browse the repository at this point in the history
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](linebender/parley#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.
  • Loading branch information
xStrom authored Nov 21, 2024
1 parent 7257b65 commit 8ec05a3
Showing 1 changed file with 24 additions and 4 deletions.
28 changes: 24 additions & 4 deletions content/wiki/canonical_lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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.
Expand Down

0 comments on commit 8ec05a3

Please sign in to comment.