Skip to content
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

Warn the user about side effects when creating empty arrays and vectors #6439

Closed
woodruffw opened this issue Dec 10, 2020 · 3 comments · Fixed by #12449
Closed

Warn the user about side effects when creating empty arrays and vectors #6439

woodruffw opened this issue Dec 10, 2020 · 3 comments · Fixed by #12449
Assignees
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy

Comments

@woodruffw
Copy link

woodruffw commented Dec 10, 2020

What it does

Both the vec![] macro and the array bulk-initializer syntax ([T; N]) allow the programmer to initialize an empty container, i.e. one that contains no copies of T.

However, both syntaxes still invoke T at least once, meaning that code like this:

let empty = vec![something(); 0];

can have unexpected side effects when something is called.

Here's a proof of concept with both a vector and an array:

fn side_effect() -> String {
    println!("side effect!");

    "foo".into()
}

fn main() {
    println!("before!");

    let x = vec![side_effect(); 0];
    
    let y = [side_effect(); 0];

    println!("{:?}, {:?}", x, y);
}

produces:

before!
side effect!
side effect!
[], []

What does this lint do?

Warns the user that empty initializations of vectors and arrays can cause side effects.

Categories (optional)

  • Kind: correctness

Justification: T's initialization is useless, since the user can't access it (they're declaring an empty container, after all) and it ends up being dropped immediately anyways.

What is the advantage of the recommended code over the original code

Avoids unexpected side effects and avoids a useless allocation.

Drawbacks

None.

Example

let x = vec![thing(); 0];

Could be written as:

let x = vec![];
@jyn514
Copy link
Member

jyn514 commented Dec 11, 2020

This is a change in behavior; if there's a suggestion at all, it should be drop(thing()); let x = vec![]. But I agree it's good to warn here.

@woodruffw
Copy link
Author

This is a change in behavior; if there's a suggestion at all, it should be drop(thing()); let x = vec![]

I agree that it's a change in behavior, but IMO there's precedent in clippy for recommending similar changes: clippy recommends closure-style APIs for things like Option and Result, even though naively rewriting ok_or to ok_or_else can cause a removal of side effects.

But I recognize that this is a different case; just wanted to point out a potential parallel 😄

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 14, 2020
…side-effects, r=dtolnay

doc(array,vec): add notes about side effects when empty-initializing

Copying some context from a conversation in the Rust discord:

* Both `vec![T; 0]` and `[T; 0]` are syntactically valid, and produce empty containers of their respective types

* Both *also* have side effects:

```rust
fn side_effect() -> String {
    println!("side effect!");

    "foo".into()
}

fn main() {
    println!("before!");

    let x = vec![side_effect(); 0];

    let y = [side_effect(); 0];

    println!("{:?}, {:?}", x, y);
}
```

produces:

```
before!
side effect!
side effect!
[], []
```

This PR just adds two small notes to each's documentation, warning users that side effects can occur.

I've also submitted a clippy proposal: rust-lang/rust-clippy#6439
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Dec 14, 2020
…side-effects, r=dtolnay

doc(array,vec): add notes about side effects when empty-initializing

Copying some context from a conversation in the Rust discord:

* Both `vec![T; 0]` and `[T; 0]` are syntactically valid, and produce empty containers of their respective types

* Both *also* have side effects:

```rust
fn side_effect() -> String {
    println!("side effect!");

    "foo".into()
}

fn main() {
    println!("before!");

    let x = vec![side_effect(); 0];

    let y = [side_effect(); 0];

    println!("{:?}, {:?}", x, y);
}
```

produces:

```
before!
side effect!
side effect!
[], []
```

This PR just adds two small notes to each's documentation, warning users that side effects can occur.

I've also submitted a clippy proposal: rust-lang/rust-clippy#6439
@camsteffen camsteffen added the good-first-issue These issues are a good way to get started with Clippy label Feb 7, 2021
@Jacherr
Copy link
Contributor

Jacherr commented Nov 16, 2023

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants