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

loan paths leaking into borrowck errors #41962

Closed
Manishearth opened this issue May 12, 2017 · 17 comments
Closed

loan paths leaking into borrowck errors #41962

Manishearth opened this issue May 12, 2017 · 17 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug.

Comments

@Manishearth
Copy link
Member

pub fn main(){
    let maybe = Some(vec![true, true]);

    loop {
        if let Some(thing) = maybe {
        }
    }
}

(playpen)

gives the error

error[E0382]: use of partially moved value: `maybe`
 --> test.rs:7:30
  |
7 |         if let Some(thing) = maybe {
  |                     -----    ^^^^^ value used here after move
  |                     |
  |                     value moved here
  |
  = note: move occurs because `(maybe:std::prelude::v1::Some).0` has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `(maybe:std::prelude::v1::Some).0`
 --> test.rs:7:21
  |
7 |         if let Some(thing) = maybe {
  |                     ^^^^^ value moved here in previous iteration of loop
  |
  = note: move occurs because `(maybe:std::prelude::v1::Some).0` has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error: aborting due to 2 previous errors

(maybe:std::prelude::v1::Some).0 seems to be MIR leaking into the UI

cc @jonathandturner @eddyb

@Manishearth Manishearth added A-diagnostics Area: Messages for errors, warnings, and lints I-papercut labels May 12, 2017
@eddyb
Copy link
Member

eddyb commented May 13, 2017

I think those are just the loan paths that borrowck uses. It's not like MIR is hooked up to it.

@eddyb
Copy link
Member

eddyb commented May 13, 2017

cc @rust-lang/compiler

@nikomatsakis nikomatsakis changed the title MIR leaking into borrowck errors loan paths leaking into borrowck errors May 16, 2017
@nikomatsakis
Copy link
Contributor

Yes, this is not MIR, but just regular loan-paths. Nonetheless, a suboptimal experience. I'm not sure what's the best way to present this. Probably we would want to (at minimum) filter on paths that the user couldn't write and try a more ambiguous phrasing (e.g., 'move occurs because bound value has type Vec<u8>')

(cc @estebank)

@nikomatsakis
Copy link
Contributor

Or maybe we can give the name of the binding (when there is one)? That might take a bit of plumbing.

@estebank
Copy link
Contributor

There probably should be only one move error in this code, should't it?

Would the following output be enough (with very small changes to the existing code)?

error[E0382]: use of partially moved value: `maybe`
 --> test.rs:7:30
  |
7 |         if let Some(thing) = maybe {
  |                     -----    ^^^^^ value used here after move
  |                     |
  |                     value moved here, because it has type `std::vec::Vec<bool>`, which does't implement the `Copy` trait

error: aborting due to previous error

@arielb1
Copy link
Contributor

arielb1 commented May 18, 2017

I think so.

@nikomatsakis
Copy link
Contributor

@estebank that output looks good to me

@Mark-Simulacrum Mark-Simulacrum added C-bug Category: This is a bug. and removed I-wrong labels Jul 26, 2017
@estebank
Copy link
Contributor

estebank commented Sep 5, 2017

I'll post a PR later today, but I'm not entirely convinced with the output, too long a label in my eyes:

error[E0382]: use of partially moved value: `maybe`
 --> file.rs:5:30
  |
5 |         if let Some(thing) = maybe {
  |                     -----    ^^^^^ value used here after move
  |                     |
  |                     value moved here because it has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `maybe.0`
 --> file.rs:5:21
  |
5 |         if let Some(thing) = maybe {
  |                     ^^^^^ value moved here in previous iteration of loop because it has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error: aborting due to 2 previous errors

vs the current

error[E0382]: use of partially moved value: `maybe`
 --> file.rs:5:30
  |
5 |         if let Some(thing) = maybe {
  |                     -----    ^^^^^ value used here after move
  |                     |
  |                     value moved here
  |
  = note: move occurs because `(maybe:std::prelude::v1::Some).0` has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `(maybe:std::prelude::v1::Some).0`
 --> file.rs:5:21
  |
5 |         if let Some(thing) = maybe {
  |                     ^^^^^ value moved here in previous iteration of loop
  |
  = note: move occurs because `(maybe:std::prelude::v1::Some).0` has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error: aborting due to previous error(s)

Three other alternatives I can think of are:

error[E0382]: use of partially moved value: `maybe`
 --> file.rs:5:30
  |
5 |         if let Some(thing) = maybe {
  |                     -----    ^^^^^ value used here after move
  |                     |
  |                     value moved here
  |                     because it has type `std::vec::Vec<bool>`
  |                     which does not implement the `Copy` trait

error[E0382]: use of moved value: `maybe.0`
 --> file.rs:5:21
  |
5 |         if let Some(thing) = maybe {
  |                     ^^^^^
  |                     |
  |                     value moved here in previous iteration of loop
  |                     because it has type `std::vec::Vec<bool>`
  |                     which does not implement the `Copy` trait

error: aborting due to 2 previous errors
error[E0382]: use of partially moved value: `maybe`
 --> file.rs:5:30
  |
5 |         if let Some(thing) = maybe {
  |                     -----    ^^^^^ value used here after move
  |                     |
  |                     value moved here because it has type `std::vec::Vec<bool>`
  = note: `std::vec::Vec<bool>` does not implement the `Copy` trait

error[E0382]: use of moved value: `maybe.0`
 --> file.rs:5:21
  |
5 |         if let Some(thing) = maybe {
  |                     ^^^^^ value moved here in previous iteration of loop because it has type `std::vec::Vec<bool>`
  = note: `std::vec::Vec<bool>` does not implement the `Copy` trait

error: aborting due to 2 previous errors

and

error[E0382]: use of partially moved value: `maybe`
 --> file.rs:5:30
  |
5 |         if let Some(thing) = maybe {
  |                     -----    ^^^^^ value used here after move
  |                     |
  |                     value moved here
  |
  = note: move occurs because `maybe.0` has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `maybe.0`
 --> file.rs:5:21
  |
5 |         if let Some(thing) = maybe {
  |                     ^^^^^ value moved here in previous iteration of loop
  |
  = note: move occurs because `maybe.0` has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error: aborting due to previous error(s)

Does it make sense to refer to (maybe:std::prelude::v1::Some).0 as maybe.0? It is technically correct, if not very illuminating. Put in another way, when does (maybe:std::prelude::v1::Some).0 make sense, other than for rustc debugging output?

@arielb1
Copy link
Contributor

arielb1 commented Sep 6, 2017

@estebank

(maybe as Some).0 is the name of the path. It is what counts for move conflicts.

@eddyb
Copy link
Member

eddyb commented Sep 6, 2017

maybe.Some::0 could be an alternative - I think it works better with named fields.

@estebank
Copy link
Contributor

estebank commented Sep 7, 2017

@arielb1 is there any reason why LpDowncast types would be presented (x as full::path::to::Enum).0 in append_loan_path_to_string and (x:full::path::to::Enum).0 in append_autoderefd_loan_path_to_string?

I'll leave changing the presentation of a shorter path using the local scope's identifier to the purview of #21934.

@arielb1
Copy link
Contributor

arielb1 commented Sep 7, 2017

@estebank

No. Maybe we should just change both (and the relevant MIR case) to x.$variantname::0? We don't ever need to list the full path - variant names are guaranteed to be distinct.

@estebank
Copy link
Contributor

estebank commented Sep 7, 2017

@arielb1 Shouldn't it be x::$variantname.0, x:$variantname.0 or x::$variantname::0 instead of x.$variantname::0?

@eddyb
Copy link
Member

eddyb commented Sep 7, 2017

@estebank x::$variantname doesn't make sense because it implies x is some sort of module or type, but it's not. : is basically as by another name.
The reason I suggested x.$variantname::$field is because Variant::field looks like a path to the field definition, and so accessing that as a field in x makes some sense.

bors added a commit that referenced this issue Jan 4, 2018
Reword reason for move note

On move errors, when encountering an enum variant, be more ambiguous and do not refer to the type on the cause note, to avoid referring to `(maybe as std::prelude::v1::Some).0`, and instead refer to `the value`.

Sidesteps part of the problem with #41962:

```
error[E0382]: use of partially moved value: `maybe`
 --> file.rs:5:30
  |
5 |         if let Some(thing) = maybe {
  |                     -----    ^^^^^ value used here after move
  |                     |
  |                     value moved here
  = note: move occurs because the value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `(maybe as std::prelude::v1::Some).0`
 --> file.rs:5:21
  |
5 |         if let Some(thing) = maybe {
  |                     ^^^^^ value moved here in previous iteration of loop
  = note: move occurs because the value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error: aborting due to 2 previous errors
```

Previous discussion: #44360

r? @arielb1
pnkfelix added a commit to pnkfelix/rust that referenced this issue May 29, 2018
…to my code.

In particular, I am adding an implicit injected borrow on the pattern
matches, and when we go around the loop, the compiler is reporting
that this injected borrow is conflicting with the move of the original
value when the match succeeds.
@estebank
Copy link
Contributor

Fixed in beta:

error[E0382]: use of partially moved value: `maybe`
 --> src/main.rs:6:30
  |
6 |         if let Some(thing) = maybe {
  |                     -----    ^^^^^ value used here after move
  |                     |
  |                     value moved here
  |
  = note: move occurs because the value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

error[E0382]: use of moved value: `(maybe as std::prelude::v1::Some).0`
 --> src/main.rs:6:21
  |
6 |         if let Some(thing) = maybe {
  |                     ^^^^^ value moved here in previous iteration of loop
  |
  = note: move occurs because the value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

@estebank
Copy link
Contributor

Scratch that, didn't notice the second main message

@estebank estebank reopened this Jan 25, 2019
@estebank
Copy link
Contributor

Current output:

error[E0382]: use of moved value
 --> src/main.rs:6:21
  |
6 |         if let Some(thing) = maybe {
  |                     ^^^^^ value moved here, in previous iteration of loop
  |
  = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

6 participants