rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.51k stars 1.55k forks source link

Vec![] to slice suggestion in simple cases #13270

Open leonardo-m opened 3 months ago

leonardo-m commented 3 months ago

What it does

This lint is meant to fire only in simple cases because analyzing the problem in general looks difficult to me. But as usual with Clippy even catching the basic situations is able to find tons of cases. This lint is meant to suggest the programmer to replace some useless vec![] allocations with regular slice literals that are short enough to avoid stack problems.

Advantage

We avoid heap allocations, LLVM generates less code, the compilation time probably decreases as well, perhaps the executable binary decreases in size a bit.

Drawbacks

We could change the meaning of the program. A Vec is a more flexible data structure compared to a slice. This lint could confuse Rust novices a bit.

Example

This code derives from the code of issue #13268.

#![warn(clippy::all)]
#![warn(clippy::nursery)]
#![warn(clippy::pedantic)]

use std::cmp::PartialOrd;

pub fn bubble_sort<T: PartialOrd>(items: &mut [T]) {
    if items.is_empty() {
        return;
    }
    let mut n = items.len();

    loop {
        let mut swapped = false;
        for i in 1 .. n {
            if items[i - 1] > items[i] {
                items.swap(i - 1, i);
                swapped = true;
            }
        }
        n -= 1;
        if !swapped { break; }
    }
}

fn main() {
    let mut test_cases = vec![
        vec![],
        vec![0_u32],
        vec![14, 4, 0, 3, 10, 6, 5, 17, 11, 19, 14, 19, 16, 0, 5, 12, 15, 2, 19, 0],
        vec![10, 15, 2, 16, 7, 14, 6, 13, 15, 0, 10, 6, 19, 2, 14, 17, 19, 4, 9, 15],
        vec![19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0],
        vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19]
    ];

    for data in &mut test_cases {
        bubble_sort(data);
        assert!(data.is_sorted());
    }
}

Could be written as:

#![warn(clippy::all)]
#![warn(clippy::nursery)]
#![warn(clippy::pedantic)]

use std::cmp::PartialOrd;

pub fn bubble_sort<T: PartialOrd>(items: &mut [T]) {
    if items.is_empty() {
        return;
    }
    let mut n = items.len();

    loop {
        let mut swapped = false;
        for i in 1 .. n {
            if items[i - 1] > items[i] {
                items.swap(i - 1, i);
                swapped = true;
            }
        }
        n -= 1;
        if !swapped { break; }
    }
}

fn main() {
    let mut test_cases = [
        &mut [][..],
        &mut [0_u32],
        &mut [14, 4, 0, 3, 10, 6, 5, 17, 11, 19, 14, 19, 16, 0, 5, 12, 15, 2, 19, 0],
        &mut [10, 15, 2, 16, 7, 14, 6, 13, 15, 0, 10, 6, 19, 2, 14, 17, 19, 4, 9, 15],
        &mut [19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0],
        &mut [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19]
    ];

    for data in &mut test_cases {
        bubble_sort(data);
        assert!(data.is_sorted());
    }
}
Alexendoo commented 3 months ago

clippy::useless_vec catches some cases, but the example is not that simple

leonardo-m commented 2 months ago

See also issue #13202 , the less heap allocations, the better.