Skip to content

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

Merged
merged 1 commit into from
May 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1002,7 +1002,7 @@ The order of associated items in traits.
The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by
reference.

**Default Value:** `target_pointer_width * 2`
**Default Value:** `target_pointer_width`

---
**Affected lints:**
Expand Down
2 changes: 1 addition & 1 deletion clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ define_Conf! {
trait_assoc_item_kinds_order: SourceItemOrderingTraitAssocItemKinds = DEFAULT_TRAIT_ASSOC_ITEM_KINDS_ORDER.into(),
/// The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by
/// reference.
#[default_text = "target_pointer_width * 2"]
#[default_text = "target_pointer_width"]
#[lints(trivially_copy_pass_by_ref)]
trivial_copy_size_limit: Option<u64> = None,
/// The maximum complexity a type can have
Expand Down
22 changes: 6 additions & 16 deletions clippy_lints/src/pass_by_ref_or_value.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::{cmp, iter};

use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
Expand All @@ -20,6 +18,7 @@ use rustc_middle::ty::{self, RegionKind, TyCtxt};
use rustc_session::impl_lint_pass;
use rustc_span::def_id::LocalDefId;
use rustc_span::{Span, sym};
use std::iter;

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

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

Self {
ref_min_size,
Expand Down
179 changes: 179 additions & 0 deletions tests/ui/trivially_copy_pass_by_ref.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
//@normalize-stderr-test: "\(\d+ byte\)" -> "(N byte)"
//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: N byte)"
#![deny(clippy::trivially_copy_pass_by_ref)]
#![allow(
clippy::disallowed_names,
clippy::extra_unused_lifetimes,
clippy::needless_lifetimes,
clippy::needless_pass_by_ref_mut,
clippy::redundant_field_names,
clippy::uninlined_format_args
)]

#[derive(Copy, Clone)]
struct Foo(u32);

#[derive(Copy, Clone)]
struct Bar([u8; 24]);

#[derive(Copy, Clone)]
pub struct Color {
pub r: u8,
pub g: u8,
pub b: u8,
pub a: u8,
}

struct FooRef<'a> {
foo: &'a Foo,
}

type Baz = u32;

fn good(a: &mut u32, b: u32, c: &Bar) {}

fn good_return_implicit_lt_ref(foo: &Foo) -> &u32 {
&foo.0
}

#[allow(clippy::needless_lifetimes)]
fn good_return_explicit_lt_ref<'a>(foo: &'a Foo) -> &'a u32 {
&foo.0
}

fn good_return_implicit_lt_struct(foo: &Foo) -> FooRef {
FooRef { foo }
}

#[allow(clippy::needless_lifetimes)]
fn good_return_explicit_lt_struct<'a>(foo: &'a Foo) -> FooRef<'a> {
FooRef { foo }
}

fn bad(x: u32, y: Foo, z: Baz) {}
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by

impl Foo {
fn good(self, a: &mut u32, b: u32, c: &Bar) {}

fn good2(&mut self) {}

fn bad(self, x: u32, y: Foo, z: Baz) {}
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by

fn bad2(x: u32, y: Foo, z: Baz) {}
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by

fn bad_issue7518(self, other: Self) {}
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if
}

impl AsRef<u32> for Foo {
fn as_ref(&self) -> &u32 {
&self.0
}
}

impl Bar {
fn good(&self, a: &mut u32, b: u32, c: &Bar) {}

fn bad2(x: u32, y: Foo, z: Baz) {}
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if
//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if
}

trait MyTrait {
fn trait_method(&self, foo: Foo);
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if
}

pub trait MyTrait2 {
fn trait_method2(&self, color: &Color);
}

trait MyTrait3 {
#[expect(clippy::trivially_copy_pass_by_ref)]
fn trait_method(&self, foo: &Foo);
}

// Trait impls should not warn
impl MyTrait3 for Foo {
fn trait_method(&self, foo: &Foo) {
unimplemented!()
}
}

mod issue3992 {
pub trait A {
#[allow(clippy::trivially_copy_pass_by_ref)]
fn a(b: &u16) {}
}

#[allow(clippy::trivially_copy_pass_by_ref)]
pub fn c(d: &u16) {}
}

mod issue5876 {
// Don't lint here as it is always inlined
#[inline(always)]
fn foo_always(x: &i32) {
println!("{}", x);
}

#[inline(never)]
fn foo_never(x: i32) {
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
println!("{}", x);
}

#[inline]
fn foo(x: i32) {
//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by
println!("{}", x);
}
}

fn ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> {
Some(x)
}

fn ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> {
Some(x)
}

fn with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 {
if true { x } else { y }
}

async fn async_implicit(x: &u32) -> &u32 {
x
}

async fn async_explicit<'a>(x: &'a u32) -> &'a u32 {
x
}

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

fn return_ptr(x: &u32) -> *const u32 {
x
}

fn return_field_ptr(x: &(u32, u32)) -> *const u32 {
&x.0
}

fn return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 {
core::ptr::addr_of!(x.0)
}
60 changes: 24 additions & 36 deletions tests/ui/trivially_copy_pass_by_ref.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
//@normalize-stderr-test: "\(\d+ byte\)" -> "(N byte)"
//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: 8 byte)"
//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: N byte)"
#![deny(clippy::trivially_copy_pass_by_ref)]
#![allow(
clippy::disallowed_names,
clippy::extra_unused_lifetimes,
clippy::needless_lifetimes,
clippy::needless_pass_by_ref_mut,
clippy::redundant_field_names,
clippy::uninlined_format_args,
clippy::needless_pass_by_ref_mut
clippy::uninlined_format_args
)]
//@no-rustfix

#[derive(Copy, Clone)]
struct Foo(u32);

Expand Down Expand Up @@ -90,21 +91,26 @@ impl Bar {
}

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

pub trait MyTrait2 {
fn trait_method2(&self, _color: &Color);
fn trait_method2(&self, color: &Color);
}

trait MyTrait3 {
#[expect(clippy::trivially_copy_pass_by_ref)]
fn trait_method(&self, foo: &Foo);
}

impl MyTrait for Foo {
fn trait_method(&self, _foo: &Foo) {
// Trait impls should not warn
impl MyTrait3 for Foo {
fn trait_method(&self, foo: &Foo) {
unimplemented!()
}
}

#[allow(unused_variables)]
mod issue3992 {
pub trait A {
#[allow(clippy::trivially_copy_pass_by_ref)]
Expand Down Expand Up @@ -135,57 +141,39 @@ mod issue5876 {
}
}

fn _ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> {
fn ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> {
Some(x)
}

#[allow(clippy::needless_lifetimes)]
fn _ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> {
fn ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> {
Some(x)
}

fn _with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 {
fn with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 {
if true { x } else { y }
}

async fn _async_implicit(x: &u32) -> &u32 {
async fn async_implicit(x: &u32) -> &u32 {
x
}

#[allow(clippy::needless_lifetimes)]
async fn _async_explicit<'a>(x: &'a u32) -> &'a u32 {
async fn async_explicit<'a>(x: &'a u32) -> &'a u32 {
x
}

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

fn _return_ptr(x: &u32) -> *const u32 {
fn return_ptr(x: &u32) -> *const u32 {
x
}

fn _return_field_ptr(x: &(u32, u32)) -> *const u32 {
fn return_field_ptr(x: &(u32, u32)) -> *const u32 {
&x.0
}

fn _return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 {
fn return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 {
core::ptr::addr_of!(x.0)
}

fn main() {
let (mut foo, bar) = (Foo(0), Bar([0; 24]));
let (mut a, b, c, x, y, z) = (0, 0, Bar([0; 24]), 0, Foo(0), 0);
good(&mut a, b, &c);
good_return_implicit_lt_ref(&y);
good_return_explicit_lt_ref(&y);
bad(&x, &y, &z);
foo.good(&mut a, b, &c);
foo.good2();
foo.bad(&x, &y, &z);
Foo::bad2(&x, &y, &z);
bar.good(&mut a, b, &c);
Bar::bad2(&x, &y, &z);
foo.as_ref();
}
Loading