Lack of `mut` in bindings as a deny-by-default lint

I think that IDEs just should automatically insert/remove mut annotations similarily to use statements

I've made that feature request for rust-analyzer, but it has been rejected that it would be "too weird" and if Rust insists mut on being explicitly (manually) specified, then rust-analyzer won't undo this policy. So I think Rust first needs to change its policy (e.g. by introducing the allow support!)

1 Like

I don't know what's the point of posting this as /sarcastic, but since you've posted it anyway, I'll add that I think it's fine, because:

  • it's not a syntax that someone would write by accident, so it still catches a case of "I intended to write in single-assignment style, but accidentally overwrote the value" mistake.

  • There's still a mut at the point of reassignment, so it's still locally explicit.

  • Shadowing exists, and can't be changed, so it's impossible to have any let rule without exceptions. Whatever previous let allows/forbids, a next one can override anyway (let x = 2).

1 Like

Because - obviously - I wouldn't actually propose that the Rust compiler should suggest

let x = 1;
*&mut x = 2;

rather than

let mut x = 1;
x = 2;

I agree. My remark was not intended as a serious counterpoint and mostly as a reminder that you potentially forgot (or at least didn't mention) that mutable references do support simple reassignment, too, when you discussed the "workaround" of using mem::replace. Using mem::replace is not necessary; you'd only need it if you don't want to drop the previous value (but in that case, you'll need mem::replace for a let mut variable as well Edit: nevermind, that's wrong).


I personally don't see shadowing as an exception, but I won't repeat going into this after I've already expressed my point on shadowing twice in this thread.

These are not the same. Consider:

fn main() {
    let mut sum = 0;
    for i in 1..=10 {
        sum = sum + i;
    }
    println!("correct sum is {}", sum);
    
    // wrong
    let sum = 0;
    for i in 1..=10 {
        let sum = sum + i;
    }
    println!("wrong sum is {}", sum);
}
1 Like

Note that shadowing is not the same as overwriting a mutable variable. Macros can refer to the shadowed variable:

let foo = 42;
macro_rules! old_foo {
    () => { foo }
}
let foo = 0;
assert_eq!(old_foo!(), 42);

Edit: accidentally posted before I was done.

2 Likes

Happened yesterday:

3 Likes

I see in this thread a lot of false claims (starting with the OP) that mut doesn't affect Rust's memory model. It is quite easy to falsify them: run the following code with Miri:

fn main() {
    let x = 3u32;
    let p = std::ptr::addr_of!(x) as *mut u32;
    unsafe { std::ptr::write(p, 5u32); }
    println!("{}", x);
}
error: Undefined Behavior: no item granting write access to tag <untagged> at alloc1011 found in borrow stack.
   --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:892:9
    |
892 |         copy_nonoverlapping(&src as *const T, dst, 1);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item granting write access to tag <untagged> at alloc1011 found in borrow stack.
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
            
    = note: inside `std::ptr::write::<u32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:892:9
note: inside `main` at src/main.rs:4:14
   --> src/main.rs:4:14
    |
4   |     unsafe { std::ptr::write(p, 5u32); }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:123:18
    = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:145:18
    = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
    = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
    = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
    = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
    = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:128:48
    = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
    = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
    = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
    = note: inside `std::rt::lang_start_internal` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:128:20
    = note: inside `std::rt::lang_start::<()>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:144:17

error: aborting due to previous error

I.e. the presence-absense of mut absolutely does affect the semantics of unsafe code, and thus can neither be omitted nor made optional. Similar considerations apply to & references.

The semantics are the following: the memory aliased by an immutable binding cannot be mutated, except the parts that are wrapped in an UnsafeCell. So simply looking at the type definition and recursively considering the component types is enough to give a certain answer to the question "can the value be changed".

This applies only to the specific memory aliased by the binding. If that memory contains pointers or references, then the above considerations say nothing about the mutability of their poinee's memory. With references one can apply Rust's usual rules to deduce transitive mutability, while with pointers the implementor is free to provide any API they want via unsafe code.

So the comments in the vein of "why can I do let x: &mut T = ...; *x = foo();" are besides the point. The immutability of x means that its value is immutable, i.e. the pointer cannot change, but there were never any guarantees about the pointed-to memory.

4 Likes

I'm not entirely sure what exactly your code example is supposed to falsify, as you will get the exact same miri error when you use let mut instead of let.

2 Likes

If you use the let mut binding, then you will be able to use &mut or std::ptr::addr_of_mut! to get directly a mutable pointer. Without a mut bindings both of those possibilities are statically excluded by the compiler, which wouldn't be the case if mut was optional.

In fact, I'm surprised that let mut x = 0; let p = ptr::addr_of!(x) as *mut _; results in an error. I have always believed (and I believe I have seen it somewhere in the docs) that the difference between *const and *mut is purely lint-level, and one can always freely cast between them without a change in semantics (but with care, since doing a write through a pointer may violate other memory model requirements). Your example seems to indicate that Miri tacks the difference between *const and *mut, but this is unexpected. The Stacked Borrows paper doesn't even mention *const and talks about pointers in general as if they are always *mut, so I have assumed that it doesn't discriminate between them. It also doesn't talk about the mutability of local variables, even though it should also affect optimizations and though it talks about mutability via shared & references. @RalfJung Is it an omission in the paper or a bug in Miri?

Perhaps the issue is that ptr::addr_of! creates some sort of temporary immutable reference, which is also surprising to me, since AFAIK the point of that macro (and its inner &raw references) is to be able to just get a pointer to the memory without any extra liveness or mutability guarantees, same as the address-of operator in C/C++.

Regardless, my point is that it is false that mutability isn't tracked in Rust's memory model. A counterexample with & references:

fn f(x: &u32) {
    let p: *const u32 = x;
    unsafe {
        std::ptr::write(p as *mut u32, 5u32);
    }
}

fn main() {
    let x = 0u32;
    f(&x);
    println!("{}", x);
}
error: Undefined Behavior: no item granting write access to tag <untagged> at alloc1011 found in borrow stack.
   --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:892:9
    |
892 |         copy_nonoverlapping(&src as *const T, dst, 1);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item granting write access to tag <untagged> at alloc1011 found in borrow stack.
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
            
    = note: inside `std::ptr::write::<u32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:892:9
note: inside `f` at src/main.rs:4:9
   --> src/main.rs:4:9
    |
4   |         std::ptr::write(p as *mut u32, 5u32);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `main` at src/main.rs:10:5
   --> src/main.rs:10:5
    |
10  |     f(&x);
    |     ^^^^^

Here we can observe that & does indeed guarantee the immutability of the referenced value. If we use a type with interior mutability, this is no longer true. Example:

use std::cell::UnsafeCell;

fn f(x: &UnsafeCell<u32>) {
    let p = (x as *const UnsafeCell<u32>).cast::<u32>();
    unsafe {
        std::ptr::write(p as *mut u32, 5u32);
    }
}

fn main() {
    let x = UnsafeCell::new(0u32);
    f(&x);
    println!("{}", x.into_inner());
}

Miri:

5
1 Like

Yep, that's still true; apparently, the difference between addr_of and addr_of_mut is not just whether it produces *mut T or *const T, but also whether you're allowed to mutate through this pointer, which is - as you mentioned - a separate concern.

Perhaps it's only about liveness (as well as pointer alignment), but not mutability? I'm not sure for example whether using add_of_mut (as well as subsequently reading from the resulting pointer) on a live pointer that you cannot write to is allowed. I believe I've seen some topic/discussion in the "unsafe code guidelines" repository about whether e.g. addr_of on a pointer that you can write to (even though it's *const) should preserve this property and still allow writes; this seems related to (but not the same as) the question of whether addr_of on a local variable (mut or not mut) should allow mutation.


I fully agree with this conclusion. It's also a great counterpoint against this argument of "please don't say mutable/immutable reference, only say unique/shared reference instead". This counterpoint is actually an (incorrect) oversimplification in the other direction; the reality is that &mut vs & is both about mutability and exclusivity. A &mut T is mutable and unique; and a &T is a shared reference that's also immutable with the exception that interior mutability through UnsafeCell, or behind the indirection of a raw pointer, is still allowed.

let mut vs. mut is only slightly different from &mut T vs &T, because let still allows

  • moving out of the variable, or fields of the variable
    • something that neither &mut T nor &T allow, because it requires owned access
  • re-borrowing the variable, or fields of the variable, of type &mut _
    • something that only &mut T allows, but &T doesn't, because it requires unique access, though not necessarily mutable access
5 Likes

That actually seems backwards from the case I most want Rust to prevent me from doing. If I have a let x which is not mut, and I call x.foo(), and foo wants a &mut self, I very much want Rust to require the addition of a mut to acknowledge that. I've found it genuinely useful to know that a let x cannot have &mut self methods called on it.

6 Likes

let mut does affect the memory model, but only in a rather simple and separable way- bringing this up as some kind of contradiction in the OP is overly pedantic and misses the point.

(And similarly, the comparison between &u32 and &UnsafeCell<u32> is just as irrelevant to this conversation as the one with ptr::addr_of!. The immutability of &x is a property of the borrow operation, not the binding- you will get the same Miri errors with let mut x as you saw with ptr::addr_of!.)

Overall this argument reminds me of ones about unsafe and "syntactic salt," and in my opinion is similarly a bit silly. There are two layers here, and we are conflating them: First there is Rust's memory and type safety, which is preserved by the semantics of & and &mut, where violations fundamentally lead to undefined behavior. Then there is Rust's culture of correctness, with tools that aid human reasoning but which are often optional or situational.

The checks around mut on bindings are inarguably in the second layer. Removing them does not break Rust's memory or type safety, or change what unsafe code can rely on. This layer does not have a clear theoretical boundary for what is "too much" or "too little" syntactic salt- arguments either way can only be made in terms of developer experience, not formal semantics.

(Personally I think these checks should be optional. Even if they weren't a needless speed bump, making them optional would make this distinction more clear! And Rust has a strong enough culture around the default lint levels that we wouldn't lose anything at code review time anyway.)

8 Likes

I've brought this example up before in this thread; nobody has really picked up the comparison: How do feel about type annotations on constants, statics and perhaps even functions return types. These are mandatory in Rust, yet they play no role in preventing undefined behavior.

I can bring up more examples: Exhaustive matches in Rust are mandatory, yet every match could just as well come with an implicit panicking catch-all arm. Implementing a trait requires implementing all required methods, yet leaving out a required method could as well just mean that the method get's an always-panicking default implementation. (These examples so far all all things that e.g. Haskell does differently, yet I support the decision of Rust to be more strict.) Statements (like blocks or if/match/while) that aren't terminated with a ; are required to be of type (), even though that could as well be just a warning. If a type doesn't implement UnwindSafe, that's a hard error, and you're required to manually insert an AssertUnwindSafe in an appropriate place; even though UnwindSafe is often called “basically just a lint” and it’s irrelevant for memory safety.

8 Likes

I think UnwindSafe is a bad example here, because it also conflates Rust's memory safety with a nebulous meaning of UnwindSafe, is something that complicates teaching about Rust because of the overloading of 'safe', and in fact does nothing really for memory safety. I'd dare say so far as it doesn't even accomplish its goal of making it harder to break logical invariants.

In fact, while the some of the examples could be explained by the fact that forgetting something shouldn't cause a panic, UnwindSafe is very, very similar to mut in the exact wrong way, a learning and development roadblock for marginal benefit.

While I'd gladly debate in detail (and possibly even question) the usefulness of UnwindSafe (except not necessarily in-depth in this thread, as it's rather off-topic), I personally think that mut is way more useful that just a “marginal benefit”. And also, UnwindSafe is hardly ever a development roadblock, as it only comes up when you use catch_unwind which should be super uncommon in everyday Rust code anyways.

3 Likes

To be clear, by "these checks" I was referring specifically to let mut, and it was in parentheses because my main point was just that soundness is not a concern, and this can only be decided based on developer experience.

But to these examples, I think there is a very clear line that can be drawn between two categories. Item types, mut, and () statement types are all very similar to unused variables or unused return values- they don't change program semantics, and we only push them into the actual program text to confirm programmer intent in relatively "unusual" situations. IMO they all make sense as lints so that they can be deferred during development but relied on during code review.

Panics on non-exhaustive matches or trait impls, on the other hand, introduce new program semantics to fill out missing information. (I would also include type-check and even borrow-check errors here.) They're not about double-checking programmer intent for an out-of-the-ordinary case, but about the necessity of doing something when the program runs. However, there is still a case here for some mechanism to defer them to runtime- that's what unimplemented!() and todo!() and ! are for, after all.

I think the usual reasoning about compiler errors and phases applies here. The compiler doesn't give up after one error or warning, it keeps going and analyzes the rest of the program. Similarly, just because a lint has fired is no reason to forbid a developer from runnig the program in their local environment (and seeing logs, or debugging!). As long as the error information is still available statically and enforceable on PRs, being permissive does nothing but empower the developer.

(And of course there is the alternative of having the IDE update the source automatically for similar effect- but either way the same motivation applies.)

2 Likes

I know, and I recognize that an explicit acknowledgement where mutation happens would be useful. But Rust doesn't have this feature.

Your exact example doesn't work when x is a &mut type. Rust will let you call a &mut self method on let x without mut, without requiring you to acknowledge the mutation.

I wouldn't be so bothered by let mut if it really meant mutation is happening. I like to be careful about mutability too. I like writing functional-style code and using immutable data. I'm using const in other languages. If Rust had a reliable feature for it, I'd use it, but Rust doesn't have it.

let x may or may not mutate. let mut x may or may not mutate. Both allow both cases. let mut means almost nothing, but requires to be added and removed seemingly at random when writing or refactoring code.

I'm currently refactoring a codebase, and went through this:

fn frobnicate(foo: &mut Foo) {
  let bar = foo.bar();
  bar.mutate();
}

foo was already &mut, it didn't need a mut bindings. Anything borrowed from it had to be &mut too, so it didn't need mut bindings either. I was able to write entire method with many let bindings, full of mutation, and not a single let mut.

But I've changed it to take owned value, and I had to add a mut:

fn frobnicate(mut foo: Foo) {
  let bar = foo.bar();
  bar.mutate();
}

At this point it doesn't seem too bad: the function's definition still says that Foo will be mutated. But then I've continued the ownership refactoring, and arrived at:

fn frobnicate(foo: Foo) {
  let mut bar = foo.into_bar();
  bar.mutate();
}

which required me to remove one mut and add another one in a different place. At all times it was the same method doing equivalent operations, and mutated the exact same data. But sometimes it had no mut, sometimes it had it in one place, sometimes in another. I've added match bar later, and had to remove mut on let bar, which was still mutated.

7 Likes

In this case x is not being mutated at all. Only *x is being mutated.

2 Likes

This is a rule that sounds sensible, but cannot be generalized; in fact, it only applies in the exact situation of x being of type &mut _, despite that the logic applies equally well to other types such as Box<_> (and types that are even more closely analogous to &mut, such as std::cell::RefMut<_>).

3 Likes