Invalid suggestion to introduce as_ref() for Option used in a place expression

[nagisa]

Consider the following code

struct Foo {
    val: i32
}

fn func1() {
    let mut foo = Some(Foo{val: 0});
    let ptr = &mut foo;
    ptr.unwrap().val += 1;
}

Here the diagnostic suggests to add as_ref as such:

note: this function takes ownership of the receiver `self`, which moves `*ptr`
help: consider calling `.as_ref()` to borrow the type's contents
  |
8 |     ptr.as_ref().unwrap().val += 1;
  |         +++++++++

This suggestion is incorrect – as_ref will never (AFAIK) produce a mutable reference necessary for the assignment. We should suggest as_mut where we know that a mutable reference will be required, instead.

[petrochenkov]

I've seen a PR fixing something similar (#96537), but it was closed due to inactivity.

[nagisa]

Right, originally this was an issue for wrong placement of `as_ref` which is still a bug on stable but has been fixed in nightly at some point, so I left this report as just a wrong as_ref ^^ 18 Jul 2022 20:46:40 Vadim Petrochenkov ***@***.***>:

…

I've seen a PR fixing something similar (#96537), but it was closed due to inactivity. -- Reply to this email directly or view it on GitHub: #99426 (comment) You are receiving this because you authored the thread. Message ID: ***@***.***>

[compiler-errors]

Right, originally this was an issue for wrong placement of as_ref which is still a bug on stable but has been fixed in nightly at some point,

Yeah, I fixed the .as_ref placement recently in #98603

where we know that a mutable reference will be required

I looked into this, but couldn't find out from the borrowck diagnostic machinery how to tell whether a place is being accessed mutably or not. I can take another look at this though.

@rustbot claim

[compiler-errors]

Worst case I could just change it to say ".as_ref() or .as_mut()".

[nagisa]

I looked into this, but couldn't find out from the borrowck diagnostic machinery how to tell whether a place is being accessed mutably or not. I can take another look at this though.

Ah, so I didn’t mean to imply that the relevant code knows this. A reasonable rule of thumb would be to see what context this expression is in. If it is a part of a place expression on the lhs of an assignment, as_mut is what user will need. I don’t know if this information is already available or, if not, what it would take to pass this around. And, yeah, I guess it wouldn’t be 100% right in all instances, so perhaps just saying “either-or” is good enough here.