False positive useless_vec when vec argument is used twice in a macro

[synek317]

Summary

If a Vec is passed to a macro that uses its argument more than once and one usage could indeed be replaced with an array but the other could not, the useless_vec lint triggers a false positive.

Link to the playground: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=e6a7db5f00b948def8a3a836f98ad071

Lint Name

useless_vec

Reproducer

I tried this code:

fn foo(_bar: usize, _baz: Vec<usize>) {}

macro_rules! baz {
    ($x:expr) => {{
        foo($x.iter().sum(), $x)
    }};
}

fn main() {
    baz!(vec![1])    
}

I saw this happen:

    Checking playground v0.0.1 (/playground)
warning: useless use of `vec!`
  --> src/main.rs:10:10
   |
10 |     baz!(vec![1])    
   |          ^^^^^^^ help: you can use an array directly: `[1]`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec
   = note: `#[warn(clippy::useless_vec)]` on by default

warning: `playground` (bin "playground") generated 1 warning (run `cargo clippy --fix --bin "playground"` to apply 1 suggestion)
    Finished dev [unoptimized + debuginfo] target(s) in 0.60s

I expected to see this happen:

    Checking playground v0.0.1 (/playground)
    Finished dev [unoptimized + debuginfo] target(s) in 0.35s

Version

1.76.0-nightly (2023-11-22 1e9dda77b5b8e690c7e2)

Additional Labels

No response

[ericwu17]

@rustbot claim

[ericwu17]

After diving into this for a bit, I wasn't able to figure out how to implement a fix. Could someone with more experience come advise? Thanks.

I know that clippy is run after macro expansion, and that during macro expansion, both the vec! and the baz! macro would be expanded. Here the vec! expression is an argument to the baz! macro. I'm unsure about how to detect all other places in the macro expansion that the vec! expression is being used.

@rustbot label +E-help-wanted

[y21]

There might be a better way but maybe we could have a map in the lint pass that maps vec![] expression Spans to a bool (whether we should lint) and delay linting until check_crate_post. That way we can always "undo" a lint at a later point if we find the same span twice, by just updating the bool to false. This is going off the assumption that if a $x is expanded more than once, it's source span will appear multiple times in the HIR.

• If we want to lint a vec![] expression and it's not yet in the map, insert it with the value true.
• If we see a vec![] expression that we don't want to lint, insert it with the value false (regardless of if that span already is in the map -- we don't want to lint either way)

Then, in check_crate_post we know that we've seen all vec![] expression and know about only those that are expanded at most once (those in the map with the bool set to true).

In the example in the OP, we would first see the expression $x.iter().sum() with the source span vec![1] and insert it with value true.
Then, we see the expression $x which also has the source span vec![1], at which point it will be updated to false, so we avoid linting it in check_crate_post.

Another way I can think of would be to get the enclosing block/body/item/parent of the macro and run a visitor through it to look for duplicate vec![] spans that way, but I'm not sure if that'll work in all cases

[ericwu17]

@y21 Thanks for the detailed explanation on how to approach this. I have changed the lint to call span_lint_and_sugg in check_crate_post but for some reason this breaks the assign_ops test. This test uses the line
#[allow(dead_code, unused_assignments, clippy::useless_vec)]
which allows useless_vec within the main() function. If I use
#![allow(clippy::useless_vec)]
to allow the lint for the entire file, the lint is indeed allowed. I assume that that emitting the lints in the check_crate_post phase has caused this allow to be ignored. How should I address this?

By the way, I've pushed my code to my fork at commit 0f306ac

[y21]

Right, if lint emission gets delayed to check_crate_post, we should use span_lint_hir_and_then (and pass the hir id of the vec![] expr) so that it respects lint level attributes on the right node. Otherwise it only respects lint level attributes on the node currently being checked (crate root) I think.