needless_borrows_for_generic_args on large array

[fominok]

Summary

Hello, I've noticed a Clippy warning that triggers a runtime error if I decide to follow its advice, because the borrow is actually needed to avoid putting the whole data on stack.

Lint Name

needless_borrows_for_generic_args

Reproducer

I tried this code:

static BIG: [u8; 1337420228] = [1; 1337420228];

fn main() {
    test(&BIG);
}

fn test<T: AsRef<[u8]>>(_t: T) {}

I saw this happen:

warning: the borrowed expression implements the required traits
 --> src/main.rs:4:10
  |
4 |     test(&BIG);
  |          ^^^^ help: change this to: `BIG`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
  = note: `#[warn(clippy::needless_borrows_for_generic_args)]` on by default

If I follow this advice by removing the borrow:

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
fish: Job 1, 'cargo run' terminated by signal SIGABRT (Abort)

Which is expected

Version

rustc 1.80.0 (051478957 2024-07-21)
binary: rustc
commit-hash: 051478957371ee0084a7c0913941d2a8c4757bb9
commit-date: 2024-07-21
host: x86_64-unknown-linux-gnu
release: 1.80.0
LLVM version: 18.1.7

Additional Labels

No response

[alex-semenyuk]

Might be better try to use heap allocation with Vec Instead of using a large static array.
This BIG array can be passed directly because it already implements the AsRef<[u8]> trait, so the lint output suggestion looks correct.

[fominok]

Might be better try to use heap allocation with Vec Instead of using a large static array.
This BIG array can be passed directly because it already implements the AsRef<[u8]> trait, so the lint output suggestion looks correct.

This is an example, but in the actual application it has to embed static bytes, that is why I ran into this issue

While the lint is right I'm not sure why it has to bother about value to be on the stack

[ptrca]

I'm curious. Did you use include_bytes! or something else to import those static bytes?
If it's the former, include_bytes shall return &'static [u8] which is already a reference.
And this lint shouldn't be triggered.

[fominok]

I'm curious. Did you use include_bytes! or something else to import those static bytes? If it's the former, include_bytes shall return &'static [u8] which is already a reference. And this lint shouldn't be triggered.

Thanks @faptc, That's a good point, indeed for some reason I store it as an array instead of a static slice

[rusty-snake]

Just came here to report that clippy::needless_borrows_for_generic_args should not trigger for large array where passing a reference is much more performant that copying. Before opening a duplicate, I comment here.

=> Clippy should not suggest a minor code styling suggestion if it negatively affects runtime.

See also https://rust-lang.github.io/rust-clippy/master/index.html#/large?groups=perf

[e00E]

I reported a similar issue in #9782 . I am copying my comment here.

Consider this code example:

fn foo<T: AsRef<[u8]>>(t: T) {
    println!("{}", std::mem::size_of::<T>());
    let _t: &[u8] = t.as_ref();
}

fn main() {
    let a: [u8; 100] = [0u8; 100];

    // prints 100
    foo(a);

    // prints 16
    foo(a.as_slice());

    // prints 8
    foo(&a);
}

clippy lints the line foo(&a) with the message "warning: the borrowed expression implements the required traits" and the suggestion to change &a to a.

It is wrong in general to suggest this change. This changes the semantics of the code. There might be very different implementations based on the type or a reference was intentionally used to not copy a large object into the function.

It is right to make this suggestion in specific circumstances. For example, the lint makes sense if you were passing &&a as there is no reason to pass a double reference. (You could construct a case where this is wrong too, but it is very rare and is fine to solve with an allow directive.)