Show warning only when let variable is modified

[medozs]

Thank you guys for your feedback. I think it doesn’t matter now whether it shows an error or warning since it does need to be fixed anyway. So consider this topic closed.

Hi Everyone, Regarding the previous discussion regarding the creation of new var keyword. Most of the opinions were rightfully against due to the creation of similar sounding keywords that do similar thing, in addition to the impossibility of implementing this keyword parsing-wise.

Today I have a new suggestions to discuss. I suggest that we show only warning, not error when let variable is modified to the user to replace with let mut(Or be even done automatically through lent , for blocks variables only), similar to the way let mut is treated when it’s not modified.

Compiler would show a warning to developer to fix one these warnings way or another. This would make rust familiar to developers coming from other languages.

What’s your opinion?

Thank You

[wesleywiser]

This effectively makes the mut keyword optional and makes regular variable bindings ambiguous:

let x = 1;

Is x mutable or immutable? How would one specify that x should be immutable?

[medozs]

User will need to put mut keyword in order for warning to go away. This shouldn’t apply to function parameters though.

[wesleywiser]

Warnings can be disabled with #[allow(...)]. Why is making mut optional desirable?

[medozs]

So we can allow let to be mutated without mut with this?

[wesleywiser]

That doesn’t seem desirable to me. I think most people in the Rust community feel strongly that this is the correct design. It would probably help your case if you could provide a code sample that shows why optional mut is better

[medozs]

It’s not better, and it would show a warning to developer to fix one way or another. This would make rust familiar to developers coming from other languages.

[josh]

I believe that Rust’s approach strikes a good balance between immutable-by-default and allowing mutability when desired. It’s an issue of fail-open versus fail-safe: https://en.wikipedia.org/wiki/Fail-safe . Making variables immutable by default and mutable when explicitly specified means that if you forget to write mut, the compiler will tell you and catch potential errors. Making variables mutable by default and immutable when explicitly specified means that if you forget to write const, your code will often compile anyway and fail to catch potential errors. So, Rust chose to make variables immutable by default. And in practice, a large fraction of variables in Rust code end up immutable.

Types are not optional annotations, and mutability is part of the type. The change you’re suggesting would turn mutability into an optional annotation. (And if we went that route, it wouldn’t be long before someone suggested making it possible to turn off the warning…)

We use warnings for cases where you’ve written valid code but you might potentially have an issue; for instance, if you leave a variable unused the compiler will warn about it, which might help you catch code you forgot to write, but it might also warn because you’re debugging something and commented out some code, in which case you might momentarily ignore the warning while debugging. On the other hand, mixing integer types is an error and can’t be overridden; attempting to mutably borrow a variable that you already have borrowed is an error and can’t be overridden; calling a method that expects self (ownership) when you have a reference is an error and can’t be overridden; missing a mut is an error and can’t be overridden; attempting to take a &mut of an immutable variable or call a &mut self method on an immutable variable is an error and can’t be overridden.

The error message you receive from the compiler when you don’t include the mut will tell you, unambiguously, that you need to add mut. If you already know enough to write let, then the compiler will help you realize that you need to write let mut. So, people familiar with other languages should have the necessary stepping stones to write Rust code. There’s a line between being familiar to developers of other languages, and attempting to let developers of those languages write the same code in Rust rather than helping people learn how to write Rust and learn the important ways in which Rust is intentionally different.

[ExpHP]

Okay, so look.

I think it’s as terrible of an idea as the next guy. But… can we at least stop fooling ourselves? Immutable bindings in Rust are ultimately a sham, as anyone who has written thread-safe code can tell you, and mutation-without-mut-as-a-hard-error doesn’t actually protect you from anything (aside from the raspberries blown by local passerby).

[Centril]

@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();.

But I think you are putting things too strongly. I do think the current situation does protect you from a certain class of mistakes by making you more conscious of mutation. That said, if you truly want immutable bindings, you’d need something like freeze T as we discussed at Forever immutable owned values (modulo interior mutability).

[oli-obk]

While hacking on the internals of miri I asked whether we could make miri error on

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

and @RalfJung responded something along the lines of "mut on let is just a lint".

The argument for this is that we could just add #[allow(unused_mut)] to our crates and use let mut everywhere unconditionally. The generated code would be 100% equal to the code generated currently. If something does not affect the outcome, it’s just a lint, right?

I’m not necessarily arguing for making missing mut a lint, but I can totally see the argument from a purity perspective.

[skade]

@steveklabnik recently voiced the opinion that let mut was a mistake, as you can always rebind any let to let mut, and most of the interesting properties are enforced based on & and &mut.

[oli-obk]

I have to disagree 100% with that statement. (but I’m missing context, so …)

I totally love the fact that the compiler tells me “you are modifying this variable but did not declare it as such”. This is imo exactly the same thing as “you’re writing a u64, but the variable is a u32”. It’s easily worked around, but gives you a some redundancy in declaration, thus preventing tiny mistakes from creeping in.

[H2CO3]

If something does not affect the outcome, it’s just a lint, right?

No. println!("{}", 0 as u8); and println!("{}", 0 as i64) result in two programs with the exact same behavior. Would you say that the types are “just a lint”? Of course not. Just because you can find an example where two constructs/pieces of code produce the same behavior doesn’t mean they are semantically equivalent, nor that they are equally desirable, safe, robust, powerful, or expressive.

Needless to say, effectively getting rid of immutability-by-default is a very bad idea; as immutability-by-default has been proven to be the safest option, according to the experience of the last couple of decades. Rust, as a modern language that is also primarily focused on reliability and correctness, should never make a compromise in this regard.

[oli-obk]

Just because you can find an example where two constructs/pieces of code produce the same behavior doesn’t mean they are semantically equivalent, nor that they are equally desirable, safe, robust, powerful, or expressive.

I’m arguing that you won’t find any counter example where the result changes.

Needless to say, effectively getting rid of immutability-by-default is a very bad idea; as immutability-by-default has been proven to be the safest option, according to the experience of the last couple of decades. Rust, as a modern language that is also primarily focused on reliability and correctness, should never make a compromise in this regard.

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);

[skade]

Needless to say, effectively getting rid of immutability-by-default is a very bad idea; as immutability-by-default has been proven to be the safest option, according to the experience of the last couple of decades. Rust, as a modern language that is also primarily focused on reliability and correctness, should never make a compromise in this regard.

One can still argue that Rust is mutable by default. Ownership rules explicitly allow mutation, let mut just makes the feature opt-in. There’s now issue with single-threaded, single-ownership mutability.

The problem is that shared mutable access is evil, and let mut by default would not make this situation worse. As mentioned above, going from let to let mut on an owned value is incredibly easy.

The code you present only works because 0 happens to be in the domain of both types and is trivially moved towards an example that exhibits different behaviour (println!("{}", x as u8); and println!("{}", x as i64); for 0 < x < 100000).

[H2CO3]

The compiler doesn’t know how to solve the halting problem, though, so when you are using unsafe to circumvent the type system, then all bets are off. Of course, it would be the best if that sort of error could be diagnosed — but alas, it can’t be in arbitrary situations.

[H2CO3]

One can still argue that Rust is mutable by default.

One can argue, but that argument is incorrect, because…

let mut makes the feature opt-in

– so if mutability is opt-in, then the language provides immutability as a default.

The code you present only works because 0 happens to be in the domain of both types

Yes, and likewise, the let mut argument only works if you happen to write code that is correct with mutation. If accidental mutation makes the code incorrect, then the presence or absence of mut changed the behavior (compiling and doing the wrong thing vs. not compiling).

[oli-obk]

it can’t be in arbitrary situations.

Where I discovered this was in miri. And we can detect this case with 100% guarantee within miri.

As a general lint in rustc, we could of course just do a best-effort lint for the easy cases, I agree.

The question is rather for the usafe coding guidelines WG: Should doing this be UB?

[H2CO3]

In that case, it should definitely be diagnosed in Miri. I think it should be diagnosed regardless of whether it’s deemed UB. It’s a bad (or at least not an unconditionally/obviously good) idea even if it’s not supposed to be “undefined”.