Skip to content

Commit 9fa448a

Browse files
authored
Make trivial-copy-size-limit consistently the size of the target pointer (#13319)
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)
2 parents cadf98b + ae0e3b7 commit 9fa448a

File tree

6 files changed

+251
-94
lines changed

6 files changed

+251
-94
lines changed

book/src/lint_configuration.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1026,7 +1026,7 @@ The order of associated items in traits.
10261026
The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by
10271027
reference.
10281028

1029-
**Default Value:** `target_pointer_width * 2`
1029+
**Default Value:** `target_pointer_width`
10301030

10311031
---
10321032
**Affected lints:**

clippy_config/src/conf.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,7 @@ define_Conf! {
828828
trait_assoc_item_kinds_order: SourceItemOrderingTraitAssocItemKinds = DEFAULT_TRAIT_ASSOC_ITEM_KINDS_ORDER.into(),
829829
/// The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by
830830
/// reference.
831-
#[default_text = "target_pointer_width * 2"]
831+
#[default_text = "target_pointer_width"]
832832
#[lints(trivially_copy_pass_by_ref)]
833833
trivial_copy_size_limit: Option<u64> = None,
834834
/// The maximum complexity a type can have

clippy_lints/src/pass_by_ref_or_value.rs

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::{cmp, iter};
2-
31
use clippy_config::Conf;
42
use clippy_utils::diagnostics::span_lint_and_sugg;
53
use clippy_utils::source::snippet;
@@ -20,6 +18,7 @@ use rustc_middle::ty::{self, RegionKind, TyCtxt};
2018
use rustc_session::impl_lint_pass;
2119
use rustc_span::def_id::LocalDefId;
2220
use rustc_span::{Span, sym};
21+
use std::iter;
2322

2423
declare_clippy_lint! {
2524
/// ### What it does
@@ -33,10 +32,8 @@ declare_clippy_lint! {
3332
/// registers.
3433
///
3534
/// ### Known problems
36-
/// This lint is target register size dependent, it is
37-
/// limited to 32-bit to try and reduce portability problems between 32 and
38-
/// 64-bit, but if you are compiling for 8 or 16-bit targets then the limit
39-
/// will be different.
35+
/// This lint is target dependent, some cases will lint on 64-bit targets but
36+
/// not 32-bit or lower targets.
4037
///
4138
/// The configuration option `trivial_copy_size_limit` can be set to override
4239
/// this limit for a project.
@@ -112,16 +109,9 @@ pub struct PassByRefOrValue {
112109

113110
impl PassByRefOrValue {
114111
pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self {
115-
let ref_min_size = conf.trivial_copy_size_limit.unwrap_or_else(|| {
116-
let bit_width = u64::from(tcx.sess.target.pointer_width);
117-
// Cap the calculated bit width at 32-bits to reduce
118-
// portability problems between 32 and 64-bit targets
119-
let bit_width = cmp::min(bit_width, 32);
120-
#[expect(clippy::integer_division)]
121-
let byte_width = bit_width / 8;
122-
// Use a limit of 2 times the register byte width
123-
byte_width * 2
124-
});
112+
let ref_min_size = conf
113+
.trivial_copy_size_limit
114+
.unwrap_or_else(|| u64::from(tcx.sess.target.pointer_width / 8));
125115

126116
Self {
127117
ref_min_size,
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
//@normalize-stderr-test: "\(\d+ byte\)" -> "(N byte)"
2+
//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: N byte)"
3+
#![deny(clippy::trivially_copy_pass_by_ref)]
4+
#![allow(
5+
clippy::disallowed_names,
6+
clippy::extra_unused_lifetimes,
7+
clippy::needless_lifetimes,
8+
clippy::needless_pass_by_ref_mut,
9+
clippy::redundant_field_names,
10+
clippy::uninlined_format_args
11+
)]
12+
13+
#[derive(Copy, Clone)]
14+
struct Foo(u32);
15+
16+
#[derive(Copy, Clone)]
17+
struct Bar([u8; 24]);
18+
19+
#[derive(Copy, Clone)]
20+
pub struct Color {
21+
pub r: u8,
22+
pub g: u8,
23+
pub b: u8,
24+
pub a: u8,
25+
}
26+
27+
struct FooRef<'a> {
28+
foo: &'a Foo,
29+
}
30+
31+
type Baz = u32;
32+
33+
fn good(a: &mut u32, b: u32, c: &Bar) {}
34+
35+
fn good_return_implicit_lt_ref(foo: &Foo) -> &u32 {
36+
&foo.0
37+
}
38+
39+
#[allow(clippy::needless_lifetimes)]
40+
fn good_return_explicit_lt_ref<'a>(foo: &'a Foo) -> &'a u32 {
41+
&foo.0
42+
}
43+
44+
fn good_return_implicit_lt_struct(foo: &Foo) -> FooRef {
45+
FooRef { foo }
46+
}
47+
48+
#[allow(clippy::needless_lifetimes)]
49+
fn good_return_explicit_lt_struct<'a>(foo: &'a Foo) -> FooRef<'a> {
50+
FooRef { foo }
51+
}
52+
53+
fn bad(x: u32, y: Foo, z: Baz) {}
54+
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
55+
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
56+
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
57+
58+
impl Foo {
59+
fn good(self, a: &mut u32, b: u32, c: &Bar) {}
60+
61+
fn good2(&mut self) {}
62+
63+
fn bad(self, x: u32, y: Foo, z: Baz) {}
64+
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
65+
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
66+
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
67+
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
68+
69+
fn bad2(x: u32, y: Foo, z: Baz) {}
70+
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
71+
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
72+
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
73+
74+
fn bad_issue7518(self, other: Self) {}
75+
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if
76+
}
77+
78+
impl AsRef<u32> for Foo {
79+
fn as_ref(&self) -> &u32 {
80+
&self.0
81+
}
82+
}
83+
84+
impl Bar {
85+
fn good(&self, a: &mut u32, b: u32, c: &Bar) {}
86+
87+
fn bad2(x: u32, y: Foo, z: Baz) {}
88+
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if
89+
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if
90+
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if
91+
}
92+
93+
trait MyTrait {
94+
fn trait_method(&self, foo: Foo);
95+
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if
96+
}
97+
98+
pub trait MyTrait2 {
99+
fn trait_method2(&self, color: &Color);
100+
}
101+
102+
trait MyTrait3 {
103+
#[expect(clippy::trivially_copy_pass_by_ref)]
104+
fn trait_method(&self, foo: &Foo);
105+
}
106+
107+
// Trait impls should not warn
108+
impl MyTrait3 for Foo {
109+
fn trait_method(&self, foo: &Foo) {
110+
unimplemented!()
111+
}
112+
}
113+
114+
mod issue3992 {
115+
pub trait A {
116+
#[allow(clippy::trivially_copy_pass_by_ref)]
117+
fn a(b: &u16) {}
118+
}
119+
120+
#[allow(clippy::trivially_copy_pass_by_ref)]
121+
pub fn c(d: &u16) {}
122+
}
123+
124+
mod issue5876 {
125+
// Don't lint here as it is always inlined
126+
#[inline(always)]
127+
fn foo_always(x: &i32) {
128+
println!("{}", x);
129+
}
130+
131+
#[inline(never)]
132+
fn foo_never(x: i32) {
133+
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
134+
println!("{}", x);
135+
}
136+
137+
#[inline]
138+
fn foo(x: i32) {
139+
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
140+
println!("{}", x);
141+
}
142+
}
143+
144+
fn ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> {
145+
Some(x)
146+
}
147+
148+
fn ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> {
149+
Some(x)
150+
}
151+
152+
fn with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 {
153+
if true { x } else { y }
154+
}
155+
156+
async fn async_implicit(x: &u32) -> &u32 {
157+
x
158+
}
159+
160+
async fn async_explicit<'a>(x: &'a u32) -> &'a u32 {
161+
x
162+
}
163+
164+
fn unrelated_lifetimes<'a, 'b>(_x: u32, y: &'b u32) -> &'b u32 {
165+
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
166+
y
167+
}
168+
169+
fn return_ptr(x: &u32) -> *const u32 {
170+
x
171+
}
172+
173+
fn return_field_ptr(x: &(u32, u32)) -> *const u32 {
174+
&x.0
175+
}
176+
177+
fn return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 {
178+
core::ptr::addr_of!(x.0)
179+
}

tests/ui/trivially_copy_pass_by_ref.rs

Lines changed: 24 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
//@normalize-stderr-test: "\(\d+ byte\)" -> "(N byte)"
2-
//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: 8 byte)"
2+
//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: N byte)"
33
#![deny(clippy::trivially_copy_pass_by_ref)]
44
#![allow(
55
clippy::disallowed_names,
6+
clippy::extra_unused_lifetimes,
67
clippy::needless_lifetimes,
8+
clippy::needless_pass_by_ref_mut,
79
clippy::redundant_field_names,
8-
clippy::uninlined_format_args,
9-
clippy::needless_pass_by_ref_mut
10+
clippy::uninlined_format_args
1011
)]
11-
//@no-rustfix
12+
1213
#[derive(Copy, Clone)]
1314
struct Foo(u32);
1415

@@ -90,21 +91,26 @@ impl Bar {
9091
}
9192

9293
trait MyTrait {
93-
fn trait_method(&self, _foo: &Foo);
94+
fn trait_method(&self, foo: &Foo);
9495
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if
9596
}
9697

9798
pub trait MyTrait2 {
98-
fn trait_method2(&self, _color: &Color);
99+
fn trait_method2(&self, color: &Color);
100+
}
101+
102+
trait MyTrait3 {
103+
#[expect(clippy::trivially_copy_pass_by_ref)]
104+
fn trait_method(&self, foo: &Foo);
99105
}
100106

101-
impl MyTrait for Foo {
102-
fn trait_method(&self, _foo: &Foo) {
107+
// Trait impls should not warn
108+
impl MyTrait3 for Foo {
109+
fn trait_method(&self, foo: &Foo) {
103110
unimplemented!()
104111
}
105112
}
106113

107-
#[allow(unused_variables)]
108114
mod issue3992 {
109115
pub trait A {
110116
#[allow(clippy::trivially_copy_pass_by_ref)]
@@ -135,57 +141,39 @@ mod issue5876 {
135141
}
136142
}
137143

138-
fn _ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> {
144+
fn ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> {
139145
Some(x)
140146
}
141147

142-
#[allow(clippy::needless_lifetimes)]
143-
fn _ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> {
148+
fn ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> {
144149
Some(x)
145150
}
146151

147-
fn _with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 {
152+
fn with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 {
148153
if true { x } else { y }
149154
}
150155

151-
async fn _async_implicit(x: &u32) -> &u32 {
156+
async fn async_implicit(x: &u32) -> &u32 {
152157
x
153158
}
154159

155-
#[allow(clippy::needless_lifetimes)]
156-
async fn _async_explicit<'a>(x: &'a u32) -> &'a u32 {
160+
async fn async_explicit<'a>(x: &'a u32) -> &'a u32 {
157161
x
158162
}
159163

160-
fn _unrelated_lifetimes<'a, 'b>(_x: &'a u32, y: &'b u32) -> &'b u32 {
164+
fn unrelated_lifetimes<'a, 'b>(_x: &'a u32, y: &'b u32) -> &'b u32 {
161165
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
162166
y
163167
}
164168

165-
fn _return_ptr(x: &u32) -> *const u32 {
169+
fn return_ptr(x: &u32) -> *const u32 {
166170
x
167171
}
168172

169-
fn _return_field_ptr(x: &(u32, u32)) -> *const u32 {
173+
fn return_field_ptr(x: &(u32, u32)) -> *const u32 {
170174
&x.0
171175
}
172176

173-
fn _return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 {
177+
fn return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 {
174178
core::ptr::addr_of!(x.0)
175179
}
176-
177-
fn main() {
178-
let (mut foo, bar) = (Foo(0), Bar([0; 24]));
179-
let (mut a, b, c, x, y, z) = (0, 0, Bar([0; 24]), 0, Foo(0), 0);
180-
good(&mut a, b, &c);
181-
good_return_implicit_lt_ref(&y);
182-
good_return_explicit_lt_ref(&y);
183-
bad(&x, &y, &z);
184-
foo.good(&mut a, b, &c);
185-
foo.good2();
186-
foo.bad(&x, &y, &z);
187-
Foo::bad2(&x, &y, &z);
188-
bar.good(&mut a, b, &c);
189-
Bar::bad2(&x, &y, &z);
190-
foo.as_ref();
191-
}

0 commit comments

Comments
 (0)