Skip to content

vicious "add &" then "remove &" cycle of suggestions #102892

Description

@pnkfelix

Given the following code (playground):

#![allow(dead_code, unused_variables)]

use std::sync::Arc;

#[derive(Debug)]
struct A;
#[derive(Debug)]
struct B;

fn process_without_annot(arc: &Arc<(A, B)>) {
    let (a, b) = **arc; // suggests putting `&**arc` here; with that, fixed!
}

fn process_with_annot(arc: &Arc<(A, B)>) {
    let (a, b): (A, B) = **arc; // suggests putting `&**arc` here too
}

The current output is:

error[[E0507]](https://doc.rust-lang.org/nightly/error-index.html#E0507): cannot move out of an `Arc`
  --> src/lib.rs:11:18
   |
11 |     let (a, b) = **arc; // suggests putting `&**arc` here; with that, fixed!
   |          -  -    ^^^^^ help: consider borrowing here: `&**arc`
   |          |  |
   |          |  ...and here
   |          data moved here
   |
   = note: move occurs because these variables have types that don't implement the `Copy` trait

error[[E0507]](https://doc.rust-lang.org/nightly/error-index.html#E0507): cannot move out of an `Arc`
  --> src/lib.rs:15:26
   |
15 |     let (a, b): (A, B) = **arc; // suggests putting `&**arc` here too
   |          -  -            ^^^^^ help: consider borrowing here: `&**arc`
   |          |  |
   |          |  ...and here
   |          data moved here
   |
   = note: move occurs because these variables have types that don't implement the `Copy` trait

To be clear, both of the above are fine suggestions.

Here's the problem:

When you make the suggested change on the second version (the one with the type annotation in the interior of the code):

fn process_with_annot_partially_fixed(arc: &Arc<(A, B)>) {
    let (a, b): (A, B) = &**arc; // ... but suggests *removing* the `&` here!
    //          ~~~~~~
    // Shouldn't it suggest the alternative (correct) fix of changing the 
    // type annotation?
}

The current output is:

error[[E0308]](https://doc.rust-lang.org/nightly/error-index.html#E0308): mismatched types
  --> src/lib.rs:19:26
   |
19 |     let (a, b): (A, B) = &**arc; // ... but suggests *removing* the `&` here!
   |                 ------   ^^^^^^ expected tuple, found `&(A, B)`
   |                 |
   |                 expected due to this
   |
   = note:  expected tuple `(A, B)`
           found reference `&(A, B)`
help: consider removing the borrow
   |
19 -     let (a, b): (A, B) = &**arc; // ... but suggests *removing* the `&` here!
19 +     let (a, b): (A, B) = **arc; // ... but suggests *removing* the `&` here!
   |

Ideally the output should look like:

help: consider removing the borrow
   |
19 -     let (a, b): (A, B) = &**arc; // ... but suggests *removing* the `&` here!
19 +     let (a, b): (A, B) = **arc; // ... but suggests *removing* the `&` here!
   |
help: alternatively, consider changing the type annotation:
   |
19 -     let (a, b): (A, B) = **arc; // ... but suggests *removing* the `&` here!
19 +     let (a, b): &(A, B) = **arc; // ... but suggests *removing* the `&` here!
   |

(Really, for types where the right-hand side won't be able to be moved into the left-hand side, it would be better to not suggest adding the & at all. But my immediate goal is to at least include the alternative suggestion.)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-diagnosticsArea: Messages for errors, warnings, and lintsT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions