Skip to content

ptr_arg lint wrongly suggests replacing &mut Vec<_> parameters with &mut [_] if the function body uses the Vec API #8482

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

Closed
sdroege opened this issue Feb 28, 2022 · 6 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@sdroege
Copy link

sdroege commented Feb 28, 2022

Summary

The function in question takes mutable Vecs as parameters and modifies them.

Lint Name

ptr_arg

Reproducer

I tried this code:

pub fn generate_reexports(
    env: &Env,
    analysis: &analysis::object::Info,
    module_name: &str,
    contents: &mut Vec<String>,
    traits: &mut Vec<String>,
    builders: &mut Vec<String>,
) {
    let mut cfgs: Vec<String> = Vec::new();
    if let Some(cfg) = general::cfg_condition_string(analysis.cfg_condition.as_ref(), false, 0) {
        cfgs.push(cfg);
    }
    if let Some(cfg) = general::version_condition_string(env, None, analysis.version, false, 0) {
        cfgs.push(cfg);
    }
    if let Some(cfg) = general::cfg_deprecated_string(
        env,
        Some(analysis.type_id),
        analysis.deprecated_version,
        false,
        0,
    ) {
        cfgs.push(cfg);
    }

    contents.push("".to_owned());
    contents.extend_from_slice(&cfgs);
    contents.push(format!("mod {};", module_name));
    contents.extend_from_slice(&cfgs);

    contents.push(format!(
        "{} use self::{}::{};",
        analysis.visibility.export_visibility(),
        module_name,
        analysis.name,
    ));

    if analysis.need_generate_trait() {
        for cfg in &cfgs {
            traits.push(format!("\t{}", cfg));
        }
        traits.push(format!(
            "\tpub use super::{}::{};",
            module_name, analysis.trait_name
        ));
    }

    if has_builder_properties(&analysis.builder_properties) {
        for cfg in &cfgs {
            builders.push(format!("\t{}", cfg));
        }
        builders.push(format!(
            "\tpub use super::{}::{}Builder;",
            module_name, analysis.name
        ));
    }
}

I saw this happen:

warning: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
   --> src/codegen/object.rs:589:13
    |
589 |     traits: &mut Vec<String>,
    |             ^^^^^^^^^^^^^^^^ help: change this to: `&mut [String]`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg

warning: writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
   --> src/codegen/object.rs:590:15
    |
590 |     builders: &mut Vec<String>,
    |               ^^^^^^^^^^^^^^^^ help: change this to: `&mut [String]`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg

I expected to see this happen: Lint doesn't trigger

Version

rustc 1.60.0-beta.1 (0a4f984a8 2022-02-22)
binary: rustc
commit-hash: 0a4f984a87c7ba6c74ec3e78442fec955a419e32
commit-date: 2022-02-22
host: x86_64-unknown-linux-gnu
release: 1.60.0-beta.1
LLVM version: 14.0.0

Additional Labels

@rustbot label +I-suggestion-causes-error

@sdroege sdroege added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Feb 28, 2022
@rustbot rustbot added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Feb 28, 2022
@Jarcho
Copy link
Contributor

Jarcho commented Mar 1, 2022

This might be fixed by #8464. Looks like it's the same bug.

@Jarcho
Copy link
Contributor

Jarcho commented Mar 16, 2022

Just checked on the playground and as far as I can tell this no longer occurs.

@tomhoule
Copy link

tomhoule commented Apr 8, 2022

I am observing this issue in multiple files in a large project, here for example. We push to that Vec in the function body, so the suggestion is wrong.

Clippy version: clippy 0.1.60 (7737e0b 2022-04-04)

hhirtz added a commit to LIHPC-Computational-Geometry/coupe that referenced this issue Apr 8, 2022
Update to Rust 1.60 extended clippy::ptr_arg somehow and now this
triggers a false positive.

See rust-lang/rust-clippy#8482

maybe relevant PR: rust-lang/rust-clippy#8271
hhirtz added a commit to LIHPC-Computational-Geometry/coupe that referenced this issue Apr 8, 2022
Update to Rust 1.60 extended clippy::ptr_arg somehow and now this
triggers a false positive.

See rust-lang/rust-clippy#8482

maybe relevant PR: rust-lang/rust-clippy#8271
@Jarcho
Copy link
Contributor

Jarcho commented Apr 8, 2022

I don't think the fix made it into 1.60.

@Jarcho
Copy link
Contributor

Jarcho commented Jun 30, 2022

This should be fixed in 1.61

@kraktus
Copy link
Contributor

kraktus commented Oct 19, 2022

Can be closed then?

@sdroege sdroege closed this as completed Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

No branches or pull requests

5 participants