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

Detect when move of !Copy value occurs within loop and should likely not be cloned #121652

Merged
merged 5 commits into from
Mar 18, 2024

Commits on Mar 17, 2024

  1. Detect when move of !Copy value occurs within loop and should lik…

    …ely not be cloned
    
    When encountering a move error on a value within a loop of any kind,
    identify if the moved value belongs to a call expression that should not
    be cloned and avoid the semantically incorrect suggestion. Also try to
    suggest moving the call expression outside of the loop instead.
    
    ```
    error[E0382]: use of moved value: `vec`
      --> $DIR/recreating-value-in-loop-condition.rs:6:33
       |
    LL |     let vec = vec!["one", "two", "three"];
       |         --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait
    LL |     while let Some(item) = iter(vec).next() {
       |     ----------------------------^^^--------
       |     |                           |
       |     |                           value moved here, in previous iteration of loop
       |     inside of this loop
       |
    note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary
      --> $DIR/recreating-value-in-loop-condition.rs:1:17
       |
    LL | fn iter<T>(vec: Vec<T>) -> impl Iterator<Item = T> {
       |    ----         ^^^^^^ this parameter takes ownership of the value
       |    |
       |    in this function
    help: consider moving the expression out of the loop so it is only moved once
       |
    LL ~     let mut value = iter(vec);
    LL ~     while let Some(item) = value.next() {
       |
    ```
    
    We use the presence of a `break` in the loop that would be affected by
    the moved value as a heuristic for "shouldn't be cloned".
    
    Fix rust-lang#121466.
    estebank committed Mar 17, 2024
    Configuration menu
    Copy the full SHA
    14473ad View commit details
    Browse the repository at this point in the history
  2. Point at continue and break that might be in the wrong place

    Sometimes move errors are because of a misplaced `continue`, but we didn't
    surface that anywhere. Now when there are more than one set of nested loops
    we show them out and point at the `continue` and `break` expressions within
    that might need to go elsewhere.
    
    ```
    error[E0382]: use of moved value: `foo`
      --> $DIR/nested-loop-moved-value-wrong-continue.rs:46:18
       |
    LL |     for foo in foos {
       |         ---
       |         |
       |         this reinitialization might get skipped
       |         move occurs because `foo` has type `String`, which does not implement the `Copy` trait
    ...
    LL |         for bar in &bars {
       |         ---------------- inside of this loop
    ...
    LL |                 baz.push(foo);
       |                          --- value moved here, in previous iteration of loop
    ...
    LL |         qux.push(foo);
       |                  ^^^ value used here after move
       |
    note: verify that your loop breaking logic is correct
      --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17
       |
    LL |     for foo in foos {
       |     ---------------
    ...
    LL |         for bar in &bars {
       |         ----------------
    ...
    LL |                 continue;
       |                 ^^^^^^^^ this `continue` advances the loop at line 33
    help: consider moving the expression out of the loop so it is only moved once
       |
    LL ~         let mut value = baz.push(foo);
    LL ~         for bar in &bars {
    LL |
     ...
    LL |             if foo == *bar {
    LL ~                 value;
       |
    help: consider cloning the value if the performance cost is acceptable
       |
    LL |                 baz.push(foo.clone());
       |                             ++++++++
    ```
    
    Fix rust-lang#92531.
    estebank committed Mar 17, 2024
    Configuration menu
    Copy the full SHA
    78d29ad View commit details
    Browse the repository at this point in the history
  3. Add HELP to test

    estebank committed Mar 17, 2024
    Configuration menu
    Copy the full SHA
    f216bac View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    da2364d View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    3b237d7 View commit details
    Browse the repository at this point in the history