Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert "reassignment of immutable local" and "mutable borrow of immutable local" from a hard error to a deny-by-default lint #3742

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

em-tg
Copy link

@em-tg em-tg commented Dec 15, 2024

As discussed previously:

Mutation of non-mutable locals arguably resembles a lint more than a hard error. Converting it to a configurable lint has the potential to reduce friction and more clearly distinguish it from Rust's more fundamental type system restrictions.

Implementing this is less effort than you might expect: the borrow checker already has separate code paths for handling local variables. I've written a proof-of-concept for the change as part of this pull request.

Rendered

@jswrenn
Copy link
Member

jswrenn commented Dec 15, 2024

Pedagogically, this strikes me as very useful change, as it can be used to highlight the difference between what's an essential aspect of Rust's ownership system, and what's (effectively) a lint.

I expect this RFC to cause a lot of spirited discussion, but it probably shouldn't. Educators will find this useful, and it's probably the case that only a minority of Rustaceans will opt-in to allowing the lint. For the vast majority of Rust programmers, nothing will change if this RFC is accepted.

@bjorn3
Copy link
Member

bjorn3 commented Dec 15, 2024

and it's probably the case that only a minority of Rustaceans will opt-in to allowing the lint.

Even just having the option to disable this lint means that whenever I would look at code written by someone else, I would have to take into account that a non-mut variable may actually get mutated.

Also --cap-lints allow is a thing and used for non-local dependencies by default.

Educators will find this useful

Maybe we should rather have a cli flag to turn this error into a warning instead? That way arbitrary crates can't disable the error, only the end user, and it would be at least a warning and not be possible to disable entirely (unlike lints).

This flag could have more teaching related relaxations behind it. Maybe even disabling the borrow checker? (probably doing should at least force the executable to print a big warning at startup time though and maybe even every time a function where borrowck doesn't pass gets executed.) That would probably be even more contentious than just disabling the mut error though.

@jswrenn
Copy link
Member

jswrenn commented Dec 15, 2024

Even just having the option to disable this lint means that whenever I would look at code written by someone else, I would have to take into account that a non-mut variable may actually get mutated.

As the RFC observes: This is already the case. The absence of mut keyword on a local does not signify that its value isn't mutated. Mutation may occur through interior mutation and the "move" loophole. As a contributor, if you depend on the immutability of a value, you must assess both the structure and subsequent uses of the value. The RFC, if implemented, does not change this calculus.

But, stepping back: I don't think accepting this RFC creates any great drama as a third party contributor. Consider, for instance, that non_snake_case is a configurable lint. Sure, an idiosyncratic project could names its constants in lowercase and its let bindings in UPPER_CASE — arguably a far worse peril than the one posed by this RFC — but even that extreme flexibility hasn't lead to mayhem in the Rust ecosystem.

@ehuss ehuss added the T-lang Relevant to the language team, which will review and decide on the RFC. label Dec 15, 2024
@bjorn3
Copy link
Member

bjorn3 commented Dec 15, 2024

Mutation may occur through interior mutation and the "move" loophole.

I don't consider the "move loophole" to be a loophole. It still requires explicitly writing let mut for the variable you want to mutate. As for interior mutability, it is generally either pretty clear when that happens (eg Mutex::lock()) or the effect is not something that matters much (eg the refcount of an Arc when cloning)

@juntyr
Copy link
Contributor

juntyr commented Dec 15, 2024

I can see why not allowing mutation of non-mutable locals feels more like a lint, as it enforces a particular code style. However, I'm not sure if the benefits of allowing this rule to be disabled outweigh the benefits in code readability.

Especially in IDEs jumping to a variable definition is common and you usually get type hints for inferred types as well. With these, it's possible to see if the variable is mutable itself or has top-level interior mutability. However, if allowing mutation of non-mutable locals would become possible, reading code with it would require checking every use of the variable. This might lead to much of the ecosystem forbidding the lint e.g. in workspace lint configs and for some tools to flag crates which don't.

Especially if this feature would be motivated for quick code refactoring and beginners, it might be a lot easier if the lint could only be allowed/warned on a per-function scope. This would provide a much more local warning sign that code will be more difficult to read

@Lokathor
Copy link
Contributor

I don't believe that it makes code more difficult to read to begin with. I leave unused mut all over all of my projects and no one has once ever said "i can't tell what you're mutating or not!". I just add mut to a binding when rustc complains i forgot it, and then i usually leave it there forever. I used to clean up function inputs because they'd show up in rustdoc, but once that bug was fixed I don't even super care about function inputs now.

I'd rather rust not ever bug me at all about this. I'd definitely put an allow at the top of lib.rs in most, if not all, projects, and then be a happier developer for it.

@bjorn3
Copy link
Member

bjorn3 commented Dec 15, 2024

How about teaching rust-analyzer to automatically add mut to variable definitions if it sees that you write code that mutates a variable?

@Lokathor
Copy link
Contributor

like an "on_file_saved: add mut if it's missing" trigger thing? i'd be cool with that as an alternative solution

@burdges
Copy link

burdges commented Dec 15, 2024

I certianly think let mut improves readability, but also there maybe considerable room for "do what rustc, etc thinks I should do" type tools, either realtime within the IDE, or as some other pass. If you're later surprised by your own let being a let mut then you've learned something about your code.

let mut not_mutable = not_mutable;
not_mutable.push(4);
```
- But that loophole _doesn't_ work for `move` closures, which ostensibly do the same thing:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm personally not convinced by this argument: The snipped below doesn't include the mut keyword anywhere, so why would it enable mutation? (And it compiles just fine if you add let mut not_mutable = not_mutable; to the closure body).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got a bit too fancy with the IIFE. If you assign to a temporary you can see the mut:

let not_mutable = vec![1, 2, 3];
let mut f = move || { // `f` is `mut` and owns `not_mutable` after the move
    not_mutable.push(4); // still rejected by the compiler
};
f();

to compile will continue to do so after the change.

One interesting consequence of this change is that it allows mutation in places where there
previously was no syntax to allow it, for example in `ref` bindings:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax for this exists behind #![feature(mut_ref)]: mut ref mut and mut ref.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, ok. I actually don't know of any other examples off the top of my head where mut syntax is missing, so maybe the whole "syntax doesn't exist" point is actually moot. I've added a note.

}
```

# Reference-level explanation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to more concretely mention here or elsewhere all the places where mut can now be omitted.

All the examples in the RFC are for simple let bindings, but there are a more possible locations:

  • Irrefutable let bindings: let (a, b) = gen_tuple()
  • Function parameters
  • match and if let

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if being exhaustive would be worthwhile, but I added the examples you listed.

- [Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Idea.3A.20Downgrade.20.22immutable.20variable.E2.80.9C.20errors.20to.20lints)

One benefit of this change is reduced friction: adding and removing of the need for `mut` on a given local
can happen frequently when refactoring, similar to how variables and functions may change from used to unused
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be curios if there's any data backing this statement. It's been a while since I've written significant amounts of Rust, but I don't think marking bindings as mut was ever an issue or particularly disruptive.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not seen any hard data, but some people consider it a major inconvenience while others swear it's never an issue. I think it depends on:

  • The way you think through problems (writing a bunch of code first then asking the compiler if it's correct vs "conversing" with the compiler while writing)
  • Tools in use (LSP/IDE vs old-fashioned edit/compile/debug loop)
  • Overall coding style (Static single-assignment and functional vs mutation-heavy and imperative)
  • Possibly also project type (library, cli, gui, server, or game)

Comment on lines +17 to +19
This RFC proposes that the re-assignment or creation of a `&mut` to a local variable not
marked with `mut` be changed from a hard error to a deny-by-default lint diagnostic (called
`mut_non_mut` for the sake of discussion).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's an error and what's a lint is a mixture of implementation detail and user configuration. Abstracting those details away, what this RFC proposes is relaxing a forbid-by-default diagnostic to deny-by-default.

Suggested change
This RFC proposes that the re-assignment or creation of a `&mut` to a local variable not
marked with `mut` be changed from a hard error to a deny-by-default lint diagnostic (called
`mut_non_mut` for the sake of discussion).
This RFC proposes that the lint against the reassignment creation of a `&mut` to a local variable not
marked with `mut` be changed from a forbid-by-default diagnostic to a deny-by-default diagnostic (called
`mut_non_mut` for the sake of discussion).
This will allow users, if they wish, to `allow(mut_non_mut)` or `warn(mut_non_mut)`.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, but I think the existing language is more precise. Lints in rustc are defined as configurable diagnostics, so a change from an unconfigurable diagnostic to a configurable one is a change from a non-lint to a lint. I like that final sentence, though, so I added it in.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not more precise; it contains a category error. Whether or not a diagnostic is a warning or error is a mix of compiler and user configuration. It's a category error to say "we're turning an error into a lint", since lints can be (and often are) errors. A user denying or forbid'ing a lint does not make the lint cease to be a lint.

Moreover, since RFCs are for user-facing changes, they should use user-facing language. It doesn't matter to a user how the compiler technically implements a diagnostic.

(An MCP, on the other hand, should reference the implementation details of a diagnostic. It'd be totally appropriate to file an MCP proposing that the E0596 diagnostic be moved from rustc_borrowck to rustc_errors as a forbid-by-default lint, since that's not a user-facing change.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a category error to say "we're turning an error into a lint", since
lints can be (and often are) errors

Perhaps my use of the term "hard error" is too ambiguous? I was using "hard" to
mean "hard-coded"/"non-configurable," but I can see how it might be unclear.

Moreover, since RFCs are for user-facing changes, they should use user-facing
language. It doesn't matter to a user how the compiler technically implements
a diagnostic.

I believe the usage of the term "lint" in user-facing rust documentation is consistent with
the "configurable diagnostic" definition. Anecdotally, I've seen the term used that
way in various online rust spaces as well.

The actual compiler output admittedly doesn't make a strong distinction.

Copy link
Member

@RalfJung RalfJung Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is accurate to call this a forbid-by-default diagnostic. Only lints have an "X-by-default" property, but this is clearly not a lint. Also, lints are subject to --cap-lints, even when they are forbidden, so it is even directly observable that the mut requirement is not a forbid-by-default anything.

The typical term we use for these is "hard error" (which is of the same category as "lint"), so it is completely in line with standard long-standing rustc terminology to say that something is converted from a hard error to an X-by-default lint.

What's an error and what's a lint is a mixture of implementation detail and user configuration.

What's a hard error and what's a lint is very directly observable by the user, and is not subject to user configuration.

EDIT: Sorry I saw too late that the thread is locked.

- But `mut` doesn't just mean "exclusive," either: local variables are _already_ exclusive
by nature, so wouldn't need `mut` under that definition.
- If you can't get a `&mut` to a local declared without `mut`, how does `drop` do it?
- There is also the "move loophole," where moving out of a binding lets you mutate it:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another 'loophole': macro invocations: There is no way, without inspecting the definition of mystery_macro, to tell what this program will print:

let foo = 42;
mystery_macro!(foo);
println!("{foo}");

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting... I'm not sure if macros count, though, since arguably the whole point of macros is to let you cheat the system a bit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, emphatically: macros count. The point you're trying to make here is that the presence of the mut modifier on a variable does not mean that its data is immutable, nor does it mean that the name itself is free from apparent mutation. You have the whole Rust Programming Language at your disposal to make this point, without exceptions.

A strong RFC pulls out all the stops in motivating itself in its Motivation section, and offers a candid accounting of consequences in the Drawbacks section. Don't undercut the RFC in the Motivation section!

Comment on lines 240 to 242
if condition {
value // rejected due to missing semicolon: probably not worth changing.
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a candidate for "lintification". It's a type error. You can verify for yourself that Rust accepts this:

if true {
    ()
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that example is a bit wonky. A better one would be this:

{
    value    
}
let whatever = 12;

Adding a semicolon after value fixes the error, but so does adding a semicolon after the block, where we expect the semicolon to be implicit.

I've removed the example anyway, per your other recommendation.

Comment on lines 245 to 249
```rs
fn maybe_thing(i: i32) -> Option<i32> {
i // rejected due to missing `Some`: Would require code generation to convert to a lint.
}
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also a type error.

I think you should move away from using the term "lintification". A lint is a well-formedness check on a program that's non-essential for soundness. mut_non_mut — even today — falls under this definition. This RFC only proposes that it be moved from forbid-by-default (which cannot be overridden), to deny-by-default (which can be overridden).

These "Future Possibilities" are an entirely different thing, proposing type-directed syntactic sugar. This RFC is controversial enough without these (im)possibilities. I suggest you remove them altogether.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, the examples were a distraction, so I removed them. That said, they were based on points raised in the IRLO thread, which might provide better context/explanation than I did.

Comment on lines +40 to +41
- Does `mut` mean mutable? Surely not, since there exists interior mutability.
- But `mut` doesn't just mean "exclusive," either: local variables are _already_ exclusive
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given your keen points that "mut" is overloaded, perhaps the lint name "mut_non_mut" is a poor choice. Consider something like unmarked_variable_mutation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added your suggestion to the list of possible lint name options.

Comment on lines 32 to 35
and back. In this context, a fatal error for `mut_non_mut` can be a flow interruption. Yes, adding
`mut` in all the right places is ideally something you should do _eventually_, just as you should
delete any unused variables/functions, but it's not something that needs to prevent you from running
the program.
Copy link
Member

@jswrenn jswrenn Dec 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "you should do this eventually" prescriptivism is unnecessary here. As @Lokathor points out, a subset of Rust users (including myself) find this diagnostic to be pure annoyance, both bloating run-edit cycles and adding line noise. Given that this RFC will allow such users to opt-out of this annoyance, there's no need to alienate this body of potential support by saying their preference is not ideal.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tweaked the phrasing a bit. I'm still not super happy with the sentence, but it's definitely better without the "you should"s in it.

@HirschBerge
Copy link

I like being reminded of my intention of making a variable a mut when I'm rereading it. Pure and simple. Lint's are messy, and I don't want to have "one more thing" yelling at me while I'm refactoring or partly through writing s function. If I forget a let mut I want neovim to have that red squiggly there immediately so I'm forced to fix it right away. If an effort to keep code more clean and readable having another deny or allow should be minimized for something this core.

@jswrenn

This comment was marked as duplicate.

@HirschBerge

This comment was marked as duplicate.

@jswrenn

This comment was marked as duplicate.

@alice-i-cecile
Copy link

alice-i-cecile commented Dec 16, 2024

Reading the 2014 blog post linked, this proposal is distinct in an important way:

I would like to remove the distinction between immutable and mutable locals

That proposal meaningfully simplifies the language, reducing complexity for everyone. This proposal makes the language less consistent, and leaves us with a largely vestigial, optional mut keyword on locals that complains via a noisy lint if you don't follow the now unnecessary rules. I'm weakly in favor of the proposal to completely remove the distinction (less verbosity is good!), but opposed to this half-measure.

request that developers use a particular coding style, and it's not clear whether that's
warranted in this case.

# Rationale and alternatives

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section should cover why this proposal is better than simply removing the use of mut for local variables over an edition boundary.

@jswrenn

This comment was marked as duplicate.

@alice-i-cecile
Copy link

If you want to move this to a lint to allow an incremental migration to "no mut for locals", then a corresponding allow-by-default lint for "mut on locals" should be added, allowing code bases to fully migrate to a post-mut world. Without that, code bases will be left in a messy half-state.

@Yokinman
Copy link

Yokinman commented Dec 16, 2024

I think there's value in being able to read any variable declaration and know whether its stack memory can change. You can technically modify memory that it points to, but the direct memory is stable. I imagine there could even be features that cleverly take advantage of its global enforcement; perhaps a macro that messes with variable declarations.

On the other hand, the keyword can be obnoxious in certain cases. e.g. in Bevy where you have to redundantly mark queries and their items as mut:

fn system(mut items: Query<&mut Item>) {
    for mut item in &mut items {
        item.x += 1;
    }
}

This is partially cause item is actually Mut<'a, Item>, a stand-in type for &mut Item that remembers if it's dereferenced. The stack memory is modified, so it's rightly marked as mut, but it's also a confusing jump as a user when &mut Item variables don't require it.

I'm not sure if this RFC is the best answer to that problem or not, though.

Edit: Actually looking at the source code, Mut stores whether it was dereferenced behind a mutable reference as well, so its stack memory remains unchanged. In that case the mut marker there is technically a little redundant after all.

@ketsuban
Copy link
Contributor

While adding mut in all the right places is something you may want to do eventually, just as you may want to delete unused variables/functions, it's not something that needs to prevent you from running the program.

"Eventually" is a very weak deadline, and I don't trust the median programmer not to kick the can down the road indefinitely if they can disable the "noisy lint" with a single line of code. As such I oppose this RFC.

@jswrenn
Copy link
Member

jswrenn commented Dec 16, 2024

I think there's value in being able to read any variable declaration and know whether its stack memory can change. You can technically modify memory that it points to, but the direct memory is stable.

@Yokinman This is a common misconception. The mut keyword only means that a variable may be mutated, not that its value is immutable. The stack memory of a variable can change even if it's not marked mut. For example:

use std::cell::Cell;

fn main() {
    let foo = Cell::new(1);
    foo.set(2);
    assert_eq!(foo.get(), 2);
}

foo doesn't hold a pointer to somewhere else. The memory directly corresponding to foo is being changed, even though it's not declared mut.

When I cite the pedagogical benefits of this RFC, this is the sort of misconception I think it would help educators dispel.

@senekor
Copy link

senekor commented Dec 16, 2024

Why don't we go a step further and make the lint warn-by-default? To me, that seems like the only way to get the actual workflow improvement.

Before:

  • write code
  • compile
  • get error about missing mut
  • add mut
  • compile again
  • run

After:

  • write code
  • compile
  • run <-- much faster debug-run cycle! benefit scales with size of project!
  • (at the beginning of next cycle, auto-fix missing-mut-warning)

The workflow only improves if the lint is a warning that doesn't prevent compilation. So, by making the lint deny-by-default, we would be asking Rust devs to tweak the lint configuration on every project to get the best workflow. In my opinion, having good defaults is one of Rust's strenghts.

Remember that the slow debug-run cycle is one of the major complaints people have with Rust!

And the difference between warn-by-default and deny-by-default is pretty much irrelevant for code that's older than five minutes. Rust doesn't have the problem of people ignoring hundreds of compiler warnings because we have good warnings.

I'm also not very concerned about having to read other people's code. For the tiny fraction of people who will actually allow this lint, they will probably do so on a project-wide basis. So it's not like every function changes style and can trip you up. And to be honest, many Rust libraries are probably hard for me to read for a number of much more impactful factors: FFI, unsafe, procedural macros... If I had to be able to read and understand all Rust libraries I use, a missing mut would be the least of my concerns.

In summary, I see the trade off like this:

+++ faster debug-run cycle
  - a tiny fraction of my libraries will become marginally less readable at worst?

@c0repwn3r
Copy link

c0repwn3r commented Dec 16, 2024

While this RFC has good spirit, I don't think this is the right way to go about it- making mut, in effect an optional token that can be thrown in at random or not used at all with no real rhyme or reason (as people will absolutely, 100% do, if they are able to disable the error) will just result in the language being inconsistent.
Also, it feels like this is removing a focus on "correctness". mut communicates useful information, in tradeoff for like 2 seconds of my time. I, and I imagine, most people, have a good enough mental map of the code I'm writing, to the point that I don't need the compiler to tell me to add mut to know I need to add it, and if compile/dev cycle time is what you're after, removing things like this is not the way to do it

@martinmroz
Copy link

martinmroz commented Dec 16, 2024

This syntax conveys the intent of the author, and removing it removes that metadata.

@AndWass
Copy link

AndWass commented Dec 16, 2024

Why don't we go a step further and make the lint warn-by-default? To me, that seems like the only way to get the actual workflow improvement.

You are now back full circle to the C++ default that everything is mutable by default, but even worse since you have no way of communicating that something really shouldn't be mutable. This default is seen by many as the wrong default. C++ has a lot of tools that will warn that something could/should be made const, something Rust will lack. And even with those tools it is considered the wrong default by many.

This change only risks harming correctness, increase the risk of proliferation of lower quality, more bug-prone code. And this default is by many considered wrong in C++. Rust learned from that, please don't undo that lesson.

100% against!

@Yokinman
Copy link

@Yokinman This is a common misconception. The mut keyword only means that a variable may be mutated, not that its value is immutable. The stack memory of a variable can change even if it's not marked mut. For example:

use std::cell::Cell;

fn main() {
    let foo = Cell::new(1);
    foo.set(2);
    assert_eq!(foo.get(), 2);
}

foo doesn't hold a pointer to somewhere else. The memory directly corresponding to foo is being changed, even though it's not declared mut.

Interesting, I did think the value of Cell was stored on the heap or similar, but you're right. unsafe is a wacky world.

It would certainly be nice if mutable dereferencing didn't require the mut marker, cause it just feels like &mut T gets special-cased over all custom reference types.

@VivekYadav7272
Copy link

VivekYadav7272 commented Dec 16, 2024

As a student I'd like to add that I do not see any pedagogical benefit to this. Coming from other languages with const, especially pass-by-reference languages like JS, most students are already aware of how a variable itself can be immutable while the value it holds continues not to be. Ironically, this change, were it present when I first learned about Rust, would've made me more confused ("why is mut optional on the variable but not when getting a reference? why can't the compiler be okay with me omitting &mut when it's okay with let mut", etc.) This will not offer any meaningful 'convenience' to newcomers but will break a long-term tribal understanding amongst existing Rust developers.

@poday
Copy link

poday commented Dec 16, 2024

The friction is intentional. Code is read much more often than it is written, minimizing the effort when reading is a good thing. While there is an additional effort when people are first learning the language that is a very useful lesson to learn early in on. Having people consistently be aware of the ownership model helps make that effort easier and second nature.

@senekor
Copy link

senekor commented Dec 16, 2024

You are now back full circle to the C++ default that everything is mutable by default

This is simply incorrect. In C++, a lack of const indicates mutability. In Rust, a lack of mut indicates immutability. The default intent of the code is unchanged. The only difference is that instead of refusing to compile perfectly safe code, rustc will give you a kind warning that you should specify your intent more clearly.

I think the correct comparison would be a proposal to make the lint against non-snake-case identifiers a hard error that's impossible to allow. Would anyone be in favor of that? No, because it slows down the run-debug cycle unnecessarily. Although it is only a warning, the Rust ecosystem does not suffer from rampant use of non-snake-case-identifiers.

why is mut optional on the variable but not when getting a reference?

The answer to that question is a very important insight into the ownership system and how it actually contributes to safety. So, I wouldn't dismiss this question as irrelevant at all.

@ahicks92
Copy link

I don't have an opinion either way on the RFC: sometimes I really don't want to be bothered (function parameters needing mut is annoying), sometimes I really care.

I do want to push back on the idea that most people won't turn it off. If you give a new Rust programmer a way to turn this off they're going to turn it off, they're not going to learn when they should and shouldn't. Most Rust code in the future will be written by new Rust programmers, a fact true of most programming languages, but especially ones still growing in popularity. There's no reason to turn this back on once you've turned it off and turning it off is one line, assuming you don't just copy a template that has it off in the first place or whatever. People aren't going to learn mut. They'll probably just come away going "man those Rust people are annoying about lints" and just not bother.

if this is accepted it is somewhat equivalent to accepting removal of mut from the language in 5 years, sort of. That's not something I can make a technical argument for, but it's my intuition of it. I also have no real opinion on removal of mut from the language, but I do want to at least point out that it seems like this RFC fundamentally makes that decision, not the smaller decision it makes on the face of it. Eventually the new people who never learned mut are the people designing the Rust of tomorrow, and of the recent RFCs this is probably the one I'd say has the most momentum in changing their view of the language.

Now that said on a sidenote, yes you can cheat mut. But in practice I see that rarely and do it rarely. I think that's kind of beside the point, because you have to make up "technically you can" examples that mostly don't happen. Even things like Mutex require the guard to be mutable so even in such cases it's not as "spooky action at a distance" as all that.

@Kl4rry

This comment was marked as duplicate.

@BirdeeHub
Copy link

BirdeeHub commented Dec 16, 2024

Do you want let val varname = "myconst";? because this is how you get let val varname = "myconst";, because this would remove peoples ability to specify something be immutable.

@AndWass

This comment was marked as duplicate.

@VorpalBlade
Copy link

From an internal-to-the-compiler perspective this might make sense (after all, Drop does get a mutable reference to any let-bound variable, so it clearly isn't "as immutable" as, say, a const). However, that is not at all the right user incentive to present. And the fact that let isn't truly readonly is quite frankly an obscure notion that is not very relevant to those not working on the compiler implementation itself.

Having lack of mut mean that the binding is actually readonly (outside of Drop and interior mutability) makes code much easier to read. It means there are much fewer things that I have to keep in my head when reading the code.

I disagree with the RFCs statement that this would make refactoring and experimenting easier, clearly seeing which parts are mutable makes my reasoning about refactoring and writing code easier. In addition my IDE is set up to underline mutable bindings. Adding/removing mut is trivial and not a burden.

As to the "move loophole", I consider it a feature, that I can say that "in this section of this function, the binding is mutable, then it isn't any more" (I rarely see any use case for going the other way, but that could happen I suppose).

Rust is "immutable by default," and intentionally makes mutable locals the "noisier" option. This makes the hard error more ergonomically costly in Rust than it would be in other languages, since it comes up more frequently in practice

Again, I consider this a feature. It is easier to reason about code that is more limited in what it can do.

Converting the hard error to a lint, even a deny-by-default one, gives people the option to turn the lint off.

The problem is that you then have to first grep a code base for anything (Cargo.toml lint table, allow-directive) that changes that default. I would consider it extreme code smell if I came across anyone actually using this, and I would probably avoid using that crate as a dependency if I could.

@AndWass
Copy link

AndWass commented Dec 16, 2024

if this is accepted it is somewhat equivalent to accepting removal of mut from the language in 5 years, sort of. That's not something I can make a technical argument for, but it's my intuition of it.

This is a very good observation. Why should new Rust programmers that are taught Rust with the lint allowed switch it to deny when it is "easier" to just keep allowing it?

@Kinrany
Copy link

Kinrany commented Dec 16, 2024

This is very likely a bad idea for implementation reasons, but may be an interesting one: we could make mut required only in release builds.

Rust-analyzer could display the missing mut keywords as inlay hints, the same way it can show inferred variable types.

@jackh726
Copy link
Member

jackh726 commented Dec 16, 2024

This thread has gotten very hot very fast, and while I've only skimmed the comments, I don't think most are providing meaningful feedback on the RFC itself. I'm going to lock this thread for now, at least until the lang team can give a vibe check on whether or not this is insta-close.

@rust-lang rust-lang locked as too heated and limited conversation to collaborators Dec 16, 2024
em-tg and others added 2 commits December 16, 2024 20:35
Co-authored-by: Wilfred Hughes <[email protected]>
Co-authored-by: Wilfred Hughes <[email protected]>
@ChayimFriedman2

This comment was marked as duplicate.

@workingjubilee
Copy link
Member

If you are permitted to comment here while this subject is locked, please refrain from striking up further discussion until this is unlocked. It is unfair to those who are not bound equally. Further, both now and when it is unlocked, you will have to try very hard to say something new: the majority of relevant points have already been made and further reiteration of them will not improve the quality nor tone of this discussion.

@ChayimFriedman2 For instance, your remarks regarding it being about correctness were already stated by @AndWass and @c0repwn3r in a largely similar form.

@workingjubilee
Copy link
Member

I have done a pass of marking-as-duplicate a number of comments. This is not a criticism of the content of those comments per se, only that we cannot possibly discuss this with a thousand "plus one" comments. Please, unless the statement you are making is procedural with respect to the RFC and not about its contents, all further commentary on the contents of this RFC should prefer to use an inline review.

And by "discuss", I mean "even to reject it". To those who are concerned about this proposal, please understand that anyone, including you, random person of the public, can open an RFC.

This RFC will not simply be thrown in without consideration of the drawbacks, many of which have already been voiced by members of the Rust team. If you have an issue with the contents of this RFC, you will not improve things by making it harder to moderate this. The reason this RFC has to happen, and should be taken seriously, is that we need to actually have a real discussion and decision in the public record, instead of muttering in various corners about it. Thus I thank the author for having the courage to start this. As they said on the original PR:

I don't necessarily expect this PR to get merged, but: ...I implemented this change mostly for fun in my free time, and I think submitting it as a pull request and being told "no" is better than never doing anything with it.

This may remain locked for a bit longer until we determine we are sufficiently ready to moderate this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.