Allow deref assignments on variables without requiring mut keyword

[b-naber]

We currently require y in this example to be mut:

fn bar(y: RefMut<'_, u32>) {
    *y = 3;
}

It seems somewhat inconsistent to not allow this to compile given that we "require" DerefMut to only be implemented on smart pointers and given that we allow this:

fn foo(x: &mut u32) {
    *x = 3;
}

This PR would allow DerefMut::deref_mut to be called on receivers that aren't declared mut.

r? @lcnr

[rustbot]

Some changes occured in rustc_ty_utils::consts.rs

cc @BoxyUwU

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

[rustbot]

These commits modify the Cargo.lock file. Random changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
This was probably unintentional and should be reverted before this PR is merged.

If this was intentional then you can ignore this comment.

[JakobDegen]

Hmm, I don't think this is entirely consistent. If I have a r: &mut u8, then being able to deref it still does not allow me to actually modify the r itself. This change would let things like this compile though, which seems significantly different:

struct S(u8);

impl Deref for S {
    type Target = u8;
    
    fn deref(&self) -> &u8 {
        &self.0
    }
}

impl DerefMut for S {
    fn deref_mut(&mut self) -> &mut u8 {
        &mut self.0
    }
}

fn main() {
    let x = S(0);
    dbg!(x.0); // 0
    *x = 1_u8;
    dbg!(x.0); // 1
}

[b-naber]

I think this touches on the point of what Deref(-Mut) implementations are valid. In the docs it says 'DerefMut should only be implemented for smart pointers to avoid confusion.'. Should these traits even be implemented for S here? Given that we can't have a hard requirement on this, this problem might make this approach invalid, however I do still think that this new behaviour is more consistent for smart pointers that implement DerefMut. We also have types in std that aren't smart pointers and implement the deref traits, though we could in principle filter for those here to not have this functionality.

[JakobDegen]

Yeah, for smart pointers it makes sense, but I don't think there's a non-surprising way to restrict this functionality to those

[CraftSpider]

Personally, I feel strongly that the 'smart pointers' wording is, at best, misleading - even the std contains deref impls for things that directly contain the provided value (such as OnceCell), and I think not having those impls would hurt ergonomics, so it's the docs that are wrong, not the impls. I agree with the above that there's no non-surprising way to restrict this to smart pointers, and personally I think that it increases, rather than decreases, surprising behavior. &mut is a single exception to an otherwise global rule, while this would create an infinite class of exceptions, where the user can't tell whether it applies from just the type name.

[compiler-errors]

I think this at least needs a lang fcp, but I agree with @CraftSpider's perspective above.

[lcnr]

This changes the builtin DerefMut::deref_mut to not require its source to be mut.

This is consistent with mutable references: the following snippet already compiles

fn derefs(x: &mut u32) {
    *x = 1;
}

With this PR, the following now also compiles

use std::cell::RefMut;
fn derefs_ref_mut(x: RefMut<'_, u32>) {
    *x = 1;
}

The status quo feels inconsistent to me and was quite annoying when using bevy as a wrapper for &mut T (Mut) is used fairly frequently.

As stated by @JakobDegen and @CraftSpider this is however somewhat weird for things which aren't simply 'smart pointers'. There are multiple impls like this in std. This PR would therefore allow the following to compile which feels very undesirable:

fn foo(x: Vec<u32>) {
    x.sort()
}

It feels difficult to restrict this PR to only apply to smart pointers without actually increasing the complexity/inconsistency 🤔

---

Given the above, I think we should not merge this PR.

We may want to open an issue as the std comment "Because of this, Deref should only be implemented for smart pointers to avoid confusion" doesn't seem to match reality?

I can't think of any convincing argument for why *x does not require x to be mutable for mutable references 🤔 it's probably just because writing mut x: &mut u32 is annoying? 😁 The "mutability" is already part of the type name, so also stating it for the variable does not add any additional information.

If so, we may want to extend this behavior to smart pointers by adding an attribute #[clearly_a_mutable_pointer] so we don't have to repeat mut for variables when using DerefMut. I would like to have such an attribute but it is fairly close to not being worth the effort. thoughts™?

cc @rust-lang/lang @rust-lang/libs-api

[nbdd0121]

The fact &mut T doesn't require mut doesn't feel inconsistent to me -- after all, we don't need to use DerefMut to get a &mut T, it's already a &mut T to begin with! &mut T is also special enough that automatic reborrow takes place, where even for Pin<&mut T> you'd have to do .as_mut() to prevent the value from being consumed.

I don't think it makes sense to be able to get a &mut P to a P that's not defined as mut. We don't even allow mutation through a non-mut Box, despite how special Box is. The analogous situation for &mut T should be &mut &mut T -- I think it would be problematic if we can get a &mut &mut T from a &mut T.

[oli-obk]

I think everything important against this PR as written has been said. We should probably move to a lang team zulip topic to discuss the mentioned alternatives and whether anything should be done here at all.

I'll go ahead and close this PR.