Show warning only when let variable is modified

[Centril]

I’d like to emphasize that calling let pat (where pat does not include mut anywhere) immutable let bindings is a pedagogical mistake and misleading simply because the introduced bindings are not immutable (see notes on mutation by first moving).

[phaylon]

I just want to mention that the let/let mut distinction constantly catches bugs for me.

The fact that I can switch between “intended to mutate” and “intended not to mutate” is not a mistake, it’s quite the opposite. It provides clarity of intent and protects against mistakes. Even the “you said you wanted to mutate this but didn’t” lint has often caught errors in the past for me.

And I’m pretty sure I’m not “fooling myself” about this.

It’s unfortunate that “mistake” is sometimes said where “has no use to me personally” might be more appropriate.

[ExpHP]

@ExpHP I think you are mostly right in the sense that let foo = bar; isn’t actually an immutable binding and that you can mutate the contents of foo by moving ownership away from foo simply with identity(foo).mutate_and_stuff();

This isnt what I was referring to. I was referring to the fact that mutation without ever writing mut is basically the definition of a threadsafe API in Rust. (See &File, RefCell, global allocators, atomics)

[RalfJung]

If that is the case, I really want my example to be an error/lint in as many cases as can be detected

let foo = 42;
unsafe { *(&foo as *const i32 as *mut i32) } = 3; // no miri error
assert_eq!(foo, 3);

I agree this code is illegal, but that’s only tangentially related to the missing mut. The code is illegal because it is writing through a pointer obtained from &T without there being an UnsafeCell. miri should eventually be able to enforce immutability of &T, and when it does, it will error on the above code appropriately.

I’m against adding some ad-hoc thing that errors on the code above, but does not also error in the equally illegal

let mut foo = 42;
unsafe { *(&foo as *const i32 as *mut i32) } = 3; // no miri error
assert_eq!(foo, 3);

which still writes through a &T.

So, in terms of UB, I still think mut is just a lint, unlike &mut. Also, mut killed renaming &mut to &uniq, which is a shame.

But at the same time I acknowledge that mut is an extremely useful lint, so this may totally be worth it. I certainly feel the urge myself sometimes to refactor my code to avoid a let mut, and it often makes it prettier; I also look at code with let mut differently (“careful here, something imperative is going on”) and the mut really helps focusing.

So I’m pretty okay with mut as it is, but still think it should have no affect whatsoever on the actual program behavior, on whether a program is UB, or on the unsafe code guidelines. That’s what I mean by “just a lint”.

[kornel]

Arguing about semantics of “lint” and whether mut is a “lint” is silly. It doesn’t matter what definition of what word it falls under, but whether the feature is useful.

For example the machine doesn’t care about distinction between pointers and integers (e.g. BCPL didn’t, and in C 7[a] works the same way as a[7]), but our squishy brains do care, so we have separate types for them.

And we have separate kinds of bindings for variables, even though neither bindings nor variables exist in the machine code, because it helps us (me, definitely) reason about the code.

[vadimcn]

If nothing else, let bindings being immutable by default will save us from Const Nazi, who insist that const must to be peppered wherever possible during C++ code reviews.

[rpjohnst]

This came up once early on in Rust’s design: mut-on-bindings doesn’t affect memory safety, and has a subtly different meaning from mut-in-references. At the same time, the distinction between &T and &mut T is often better-explained as “shared” and “unique” rather than “immutable” and “mutable” (especially given internal mutability). So someone proposed ditching mut-on-bindings to try to make that clearer.

The ensuing reaction is known as the “mutpocalypse,” and it made basic basically the same arguments as @phaylon- even though it’s not necessary for safety, the immutable-by-default style that let mut encourages tends to prevent bugs (or at least it feels like it). So even if you disagree, really this question was already settled as far as Rust’s design goes.

[leodasvacas]

I agree on the established conclusion that let mut is worth it, simply from the practical observation that most bindings are immutable and it’s nice for mutable ones stand out when reading code. But when writing code this distinction is just annoying, I wish for an IDE with rustfix integrated that automatically inserts or removes the mut as I write the code.

[steveklabnik]

To elaborate slightly on this, it’s the same as what @RalfJung is saying. You can add mut to every binding, and nothing changes.

I personally cannot ever remember a bug that needing to add mut fixed. @phaylon do you have examples here? That’d be very helpful.

I understand the resistance to this idea, because I was that way for a long time. I think saying “it’s a mistake” is a bit strong; I think it was the right call for Rust, but for whatever comes after Rust, I hope they make the other choice regarding the Mutapocalypse. Maybe the conversation will have moved forward enough by then to make it feasible.

[phaylon]

Some cases where it helped me:

• A state machine kind of like State::VariantA { stream_a, stream_b } where both streams are event sources that somehow interact or were intermingled. Taking a state and using it as template for a new state would have introduced hard to trace errors in edge cases if it had simply allowed me to mutate both without allowing intent.
• In one situation I actually had State::TransferRest { mut a, mut b } that mistakenly modified the wrong stream. The binding quickly made me see that I marked things as mutable that I didn’t intend to mutate. Fixing the bindings and removing one mut made the code not compile and pointed me exactly to where I was messing with the wrong part.
• Parsing routines where I have let mut rest = input. The "rest doesn’t need to be mutable" warning told me I forgot to update the rest variable, which would have lead to inifinite recursion.

So, it’s not adding mut that fixes bugs, it’s that having to add mut forced me to communicate my intent which caught bugs early, or helped with the diagnosis. It’s basically a piece of inline compiler enforced per-binding documentation.

[rpjohnst]

I do think whatever comes after Rust should definitely use the “shared”/“unique” terminology. That aspect is so important to understand when “fighting the borrow checker,” and it’s (I think) the next big place we could improve things, after NLL—e.g. projecting into fields and array elements of &Cells, shared mutability with types that weren’t written with Cell fields, arena/GC-allocated graphs that can easily handle both cycles and mutation.

I also think the idea of let foo = ...; identity(foo).mutate() is a huge distraction. foo absolutely is an immutable binding, but apparently people keep thinking that “immutable binding” means “immutable object.” This is kind of the inverse of the mut/&mut confusion—the let/& confusion, so to speak. Rust does have immutable objects, but they’re controlled by &T, not by let-without-mut.

[Centril]

foo absolutely is an immutable binding

Well; it is an immutable binding in the sense that you can’t mutate the binding itself; but it isn’t a mutable binding in the sense that you can mutate the object behind it (assuming there is one…) by moving. So I don’t think it is such a distraction. This difference seems important (to me).

[ExpHP]

This I don’t get. Nobody can observe any changes by looking at foo. Who cares what happens to the value after it is moved?

Surely you would not think the same if foo were a Copy type? (and copies and moves are one and the same)

[rpjohnst]

After moving the object, it is no longer bound to the non-mut name, so the binding’s mut-ness is irrelevant.

The only way a binding’s mut-ness could possibly apply to the object itself is if it were part of its type instead of part of the binding. mut-on-arguments would have to become a public part of the signature; function return types would have to include mut-ness; non-mut bindings could never be moved into mut ones; expressions themselves would have to have mut-ness.

This is an entirely new and separate feature, of dubious benefit. So yes, it is a distraction when trying to understand existing features.

[Centril]

Surely you would not think the same if foo were a Copy type? (and copies and moves are one and the same)

That’s a great point.

[leodasvacas]

Who cares what happens to the value after it is moved?

It’s more general than that, it’s “who cares about a value after it’s last use?”, hence the discarding ownership proposals. Though I guess drop cares since technically it’s the last use. Semantically it seems fine if let bindings allowed mutation on the last use, thought it causes the same confusion as discarding ownership when you write more code and that use is no longer the last.

[steveklabnik]

Thanks for this. I’m still not 100% sure I get it, exactly, but it’s nice to know that this is helping some people, at least Gotta mull over the examples more.

[phaylon]

I can give an actual code example from a while ago. Compare the communicated amount of information in

match state {
    SelectContentState::Open { mut source, builder } => {...},
    SelectContentState::Content { source, mut stream, tag } => {...},
    SelectContentState::Close { mut source, tag } => {...},
}

with

match state {
    SelectContentState::Open { source, builder } => {...},
    SelectContentState::Content { source, stream, tag } => {...},
    SelectContentState::Close { source, tag } => {...},
}

The actual ... blocks of course contain some complicated state changes, so fully knowing if something is mutated or not might be hard to determine on a glance.

A similar thing applies to the new match rules. Given a fn get() returning a &mut Something, I prefer

match *foo.get() {
    Something::A { ref a, ref mut b } => {...},
    Something::B { ref mut a, ref b } => {...},
}

over

match foo.get() {
    Something::A { a, b } => {...},
    Something::B { a, b } => {...},
}

(Note: A get would usually be get_mut, it’s just a stand-in here for some process resulting in a mutable reference.)

[Centril]

Given a fn get() returning a &mut Something, I prefer

At some point, I think the noise becomes too much and is the way of writing flow; however, I believe:

match foo.get() {
    ref mut Something::A { a, b } => {...},
    ref mut Something::B { a, b } => {...},
}

could strike a good balance of being DRY but still somewhat explicit. (This assumes that the match binding mode is ref mut for all the things – usually you don’t mix match binding modes that much…)

This follows the principle of Lagom är bäst.

[Centril]

PS: I like the fact that you can omit the binding modes if you like to while prototyping. You can come back later once you are done and before committing to VCS and add the annotations; That way, you both get improved writing flow and readability.