Rust: Matching on a place in borrowed context should no longer suggest `ref`

[scottmcm]

https://play.rust-lang.org/?gist=a88b996b2d1c86d1f6d32eee66c70cd4&version=stable&mode=debug&edition=2015

struct MyStruct<T> {
    x: Option<T>,
}

fn foo<T>(s: &MyStruct<T>) {
    match s.x {
        Some(_a) => {}
        None => {}
    }
}

This suggests using ref:

error[E0507]: cannot move out of borrowed content
 --> src/lib.rs:8:11
  |
8 |     match s.x {
  |           ^ cannot move out of borrowed content
9 |         Some(_a) => {}
  |              -- hint: to prevent move, use `ref _a` or `ref mut _a`

With match ergonomics now stable, it would be better to instead suggest borrowing the field:

error[E0507]: cannot move out of borrowed content
 --> src/lib.rs:8:11
  |
8 |     match s.x {
  |           ^ cannot move out of borrowed content
  |           - hint: to prevent move, use `&s.x` or `&mut s.x`

To help people move away from needing to know that ref exists.

(Context that lead me to open this: https://discordapp.com/channels/273534239310479360/274215136414400513/468303728986816512)

[csmoe]

cc @ashtneoi

[ashtneoi]

Yeah, I can probably tackle this.

[ashtneoi]

For something like this...

error[E0507]: cannot move out of borrowed content
  --> src/main.rs:12:11
   |
12 | fn bar<T>(&MyStruct { x: _a }: &MyStruct<T>) { }
   |           ^^^^^^^^^^^^^^^--^^
   |           |              |
   |           |              hint: to prevent move, use `ref _a` or `ref mut _a`
   |           cannot move out of borrowed content

...should it suggest changing the type, the pattern (which might be tricky, idk), or neither?

[scottmcm]

@ashtneoi My personal goal is to have the compiler never mention ref, so I'd hope for it to suggest

fn bar<T>(MyStruct { x: _a }: &MyStruct<T>)

...but I have no idea how hard that might be.

[durka]

...Can we lint against top-level type declarations like that? It's really confusing trying to figure out the type of _a.

[ashtneoi]

Oh, one more question (sorry): In a lot of cases, NLL already has better suggestions. Should I bother fixing the non-NLL suggestions or just focus on NLL assuming it'll land sometime soon?

[scottmcm]

Oh, good point; I didn't try NLL. Only fixing the NLL ones sounds like a smart plan.

[ashtneoi]

It took a ton of reading (which is why I'm only now figuring it out), but I managed to put each binding's top-level pattern span into the LocalDecl's VarBindingForm. That means it's definitely feasible to suggest a change to the entire pattern (rather than just the binding site or the RHS value). Things should be easier from here on and I'll hopefully have a PR in a few days.

[estebank]

Current output:

error[E0507]: cannot move out of borrowed content
 --> src/lib.rs:8:11
  |
8 |     match s.x {
  |           ^^^
  |           |
  |           cannot move out of borrowed content
  |           help: consider borrowing here: `&s.x`
9 |         Some(_a) => {}
  |              -- data moved here
  |
note: move occurs because `_a` has type `T`, which does not implement the `Copy` trait
 --> src/lib.rs:9:14
  |
9 |         Some(_a) => {}
  |              ^^

error[E0507]: cannot move out of borrowed content
  --> src/lib.rs:14:11
   |
14 | fn bar<T>(&MyStruct { x: _a }: &MyStruct<T>) { }
   |           ^^^^^^^^^^^^^^^--^^
   |           |              |
   |           |              data moved here
   |           cannot move out of borrowed content
   |           help: consider removing the `&`: `MyStruct { x: _a }`
   |
note: move occurs because `_a` has type `std::option::Option<T>`, which does not implement the `Copy` trait
  --> src/lib.rs:14:26
   |
14 | fn bar<T>(&MyStruct { x: _a }: &MyStruct<T>) { }
   |                          ^^

[scottmcm]

This looks great! Thanks @ashtneoi!