Skip to content

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

Closed
@pnkfelix

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.)

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