Error: local variable is never mutated

News
,
41

You can’t be serious here. The whole problem (rather, a mild inconvenience at worst) is frankly blown out of proportions.
The compiler literally gives you the lines where you have to do a minor, no-brainer change to your code to make it compile. I think I could automate the process if I really wanted to (I don’t).

C is the language that doesn’t patronize me. It doesn’t care whether you don’t use a var, or whether it’s initialized, or whether there’s an overflow, or index out of bounds, etc. Don’t get in programmer’s way.
I used to enjoy this when I was young and thought I could do no wrong. Now I strongly prefer languages with safety nets.

And having these safety nets as an option means there will be people not using them. I’m not talking about array index checks, for example - these incur performance costs and there’s a legitimate reason to have them as an option. If those were free, I’d love them to be mandatory too.

I’m fearful of this “yeah, it’s kinda mandatory, but not really” approach, and I’m all for unused vars and var where a const could be used to be errors, period. After all, there are simple workarounds for both of them, I only wish those workarounds were more explicit, like a special keyword - that would make searching for them a breeze.

1 Like
42

This discussion comes up every time an error like this is implemented: there was a similar widespread reaction when we introduced errors for unused variables, for instance. I do understand those who think some kind of “sloppy” mode would work well, but I think it’s a flawed idea.

Firstly, it’s worth noting that Andrew’s mind probably isn’t going to change on this. This argument has been extensive, drawn-out, and gone on for years. In that time, Andrew has almost certainly heard every single argument you could possibly make, and his mind has not been changed. As BDFL/BDFN, he gets to make that call.

Now, let’s be clear: it’s pretty rare to make many changes to code during development that would trigger these errors. For sure, a block of code is an evolving thing, and I change my mind on how things should work as I write them; but it’s incredibly rare (I actually can’t think of a single time where this has happened) for me to explicitly decide a variable is going to be mutated, change my mind down the line, but not already have to modify the line it is declared on. This scenario simply doesn’t normally arise in the edit-build-run cycle, so requiring one more trivial modification (which your IDE can do for you anyway if you want it to!) doesn’t really change anything.

A tip for @gonzo and any others who have raised similar issues about commenting out blocks of code: if you want to comment out a big block of code in Zig, the closest equivalent to C’s #if 0 is to simply write if (false). It’s logically the exact same thing: what C does with the preprocessor, Zig tries to do with comptime. Not only does this nest well, it’s particularly useful in preventing errors like this from changing - the compiler doesn’t care that the code mutating your local variable is not actually analyzed, provided it exists. This is equally true for any error which uses “global information” like this - such errors are not dependant on semantic analysis, so wrapping the code in if (false) won’t introduce any error. For instance, if (false) _ = x; is still treated as a discard of x.

A big practical problem with the idea of a --sloppy mode (or similar) is - as has already been mentioned - that it can and will be found on code in the wild. This is not theoretical: look at the amount of code that exists which emits warnings on virtually every major C compiler, for instance. People will treat these “sloppy” errors just like warnings if they don’t block compilation, and when people forget to test their code without this option, code ends up on the internet with these issue. Worse still, if that’s a package, someone could use the package and not even notice that there’s an issue until they try to build their program in a release mode! Depending on how the option is implemented, the endgame here is either a. huge amounts of Zig code in the wild being useless to anything that isn’t a toy program, since it can’t be used in release mode, or b. the proliferation of --sloppy as a default, required to use large parts of the ecosystem. This code would also invariably appear in spaces like this forum, which makes helping people harder; code which makes these mistakes is naturally harder to read and understand.

That brings me on to the main, and perhaps most important, point. Errors like this do not just exist to make code easier to read - that’s a key goal, but not the only one. Another key goal is that this form of error helps prevent bugs. I really like how Andrew summarises this - in essence, Zig is designed to prevent bugs by strategically introducing friction to bug-prone areas of the language. When you consider it in this context, these errors are arguably more important during development, since that’s when you’re going to be writing code and so introducing bugs!

This is the case with unused variables for instance - having a variable which you have named but not used anywhere is a quite bug-prone pattern (it indicates that you have likely made a mistake), so Zig introduces the intentional friction of requiring a discard (_ = x) to acknowledge the intent here. This has two effects: it forces you as the author to acknowledge that you’ve used this pattern intentionally, and it highlights this fact to readers of your code (improving readability). I personally have had a significant amount of buggy code caught before I even hit build due to the existence of this error.

The new error is doing exactly the same thing. Writing var - indicating explicit intent to mutate a variable - and then never writing code which has the capability to do so is a clear indicator that you’ve gone wrong somewhere. Perhaps you changed your mind on exactly what the variable meant when writing code, or perhaps you restructured something; but more likely, you either should have written const in the first place, or you forgot to mutate something which definitely needs to be mutated. Don’t just take my word for this - when implementing this change, fixing the parts of the Zig codebase that flagged up this error identified 3 separate bugs (two in the standard library and one in the compiler). These bugs would have been tricky to spot otherwise, but were extremely obvious in the presence of this error. For instance, some Mach-O linker code initialized a var next_sym: usize = 0, and never changed it, which - even having zero familiarity with the relevant code - I could tell was clearly a bug. 3 bugs in a fairly well-audited and reviewed codebase used by a lot of people is not insignificant.

Also, as a quick closing note: const is significantly easier for the compiler to optimize. There isn’t much to say here, but the point is that a const often need not correspond to a stack allocation in the compiler’s first intermediate representation (ZIR), so the self-hosted backends can often lower it to being stored in a register even with no optimization passes whatsoever. Pretty neat!

TL;DR: this error message not only helps improve readability, but also contributes a lot to avoiding bugs in code, and is friendlier to the compiler (potentially improving runtime performance). Regardless, this issue is ultimately Andrew’s call as the BDFL, and I can gurantee you he’s heard every argument under the sun here.

16 Likes
43

I think this actually makes some of your arguments irrelevant.
If you had used this IDE feature, would you have found those 3 bugs?

Honestly, to me that seems more of a shortcoming of the current compiler implementation.
I think in the long term it is almost a necessity to find a way to apply those optimizations for variables as well. Like C/C++ compilers also do this without optimizations.

1 Like
44

I’d guess that I would still have found 1 or 2 of them, or at least they would have been much more likely to be found, because something should be const which obviously is not supposed to be const (an undefined or a 0). However, it definitely wouldn’t have been as good as manually fixing. I actually don’t like using the ZLS fixups, and turn them all off myself - I find they do indeed largely defeat the point of the errors they fix. However, my point is merely that if you really, really hate having to make these changes, then you can make your IDE do it for you, and it’s basically as if this change never happened; nothing to complain about!

I’m not saying “it’s only possible to do XYZ in codegen if you mark things const”, I’m saying “it’s easier to do XYZ”. We obviously can and will implement optimization passes which promote stack allocations to registers where possible, that’s incredibly important. However, const will remain easier for the compiler to handle, since it doesn’t need to utilize this logic at all. There is no optimization pass happening to make this efficient for consts, it’s just kind of their nature. No matter how capable the compiler is, const will always be easier for it to handle, simply because it places more constraints on its usage which allow for a simpler lowering.

4 Likes
45

This is a great tip, thanks for that @mlugg !

As you said it, in the end this is @andrewrk 's call.

Peace!

4 Likes
46

A quick side note: const is also thread-friendly.

47

I did not know that. That makes exploratory code a lot more feasible.
The new const error already helped catch a bug on the first day it was implemented, it’s great. But I remember when I started Zig, I was rapidly iterating over a particularly tricky algorithm that I was trying to implement, and the unused variable error was driving me crazy, to the point I actually gave up on the language. Next day, I saw somewhere that zls was updated and it was now fixing this automatically, so I came back. I’m so thankful for it.
Even with the autofix, it still helps me catch bugs. When I change something and I see all lines shifting down, I know zls has introduced a discard for me, and a lot of times it made me realise something was wrong.
In any case, I looked at @mlugg ’ PR, and it’s super small, 26 additions and 2 deletions. Someone more dedicated to this matter than me could easily fork Zig and undo it. Then people can use it during exploratory phases. I already have the mandatory name for the fork, and anyone is free to use it: SloppyZig.

1 Like
48

Ah, I think you’re right about that quote I was referring to. I’ll edit my original post. The part about automatically promoting pointers to const (SpexGuy did a detailed post on that) was an addendum to the concerns here but not directly related to this issue that I quoted.

1 Like
49

sorry for interfering…

  • why only “local”? are there any “global” variables in Zig? Examples?
  • a “variable” by it’s definition is something that can have various values over the time.

so, it is the matter of terms, kinda “mutable entity vs immutable entity”.

var q // mutable
let r // "don't try to change me" (c)

:upside_down_face:

50

Reading the discussion, it made me remember that somewhere there was a description about intent so in doing a search, found it in the Maintainable definition of the language reference page at the Introduction:

Precisely communicate intent to the compiler and other programmers.

So for the compiler, it’s performing the analysis and doing work. Based on the discussion we all value that work and the result. The question is whether to have a toggle switch to ignore the result.

For myself, I need all the training wheels I can get, to be honest. Whenever it shows the error, it’s as if the compiler is asking “Hey Friend, did you REALLY mean for this field to be im/mutable? because I see later down here that you never/do modify it?” which makes me look, and think yes or okay-rookie-mistake. I prefer that the compiler stop me instead of automatically changing the source, because I don’t want it to assume my intent.

1 Like
51

Did anyone ever consider something like this?

// pseudo code
fn foo() i32 {
    @allowUnmutatedLocalVariablesAllowed(true); // hypothetical
    var a: i32 = 42;
    return 42;
}

The builtin function @allowUnmutatedLocalVariablesAllowed(true|false) controls whether the compiler treats unmutated local variables as errors or not. Kind of similar in spirit to @setRuntimeSafety(true|false) or @setCold(true|false). This would make it visible in code reviews, is greppable, could be controlled by additional compiler flags (e.g. -fno-sloppy-ever or -fallow-sloppy-in-the-first-place), and would enable exploring and experimenting with temporarily disabling large chunks of code or introducing prototyping code.

1 Like
52

If you really need this you can do the equivalent on a per variable basis without any help from the compiler.

fn foo() i32 {
    var a: i32 = 42;
    allowUnmutatedLocal(&a);
    return 42;
}

inline fn allowUnmutatedLocal(_: *anyopaque) void {}

You can add your own build flag to determine whether allowUnmutatedLocal is defined or not.

I personally don’t see the point though. I’d much rather just change var to const and get on with life. All of the cases I had to fix in my own code were bugs or at very least code smells anyway.

1 Like
53

This still requires you to manually, explicitly reference every single variable where this is the case. So it does not play well in the exploration/prototyping use case.

“Just accept it and move on” is not a great basis for discussion and trying to figure out improvements. May as well say “Just stick with C and get on with life”.

3 Likes
54

I didn’t say accept it and move on though. I said fix it and move on.

The whole point is that this will make your code objectively better, so just do it. That is the opposite of accepting the status quo and sticking with C.

55

I really like this behavior of the ZIG compiler because it makes ZIG code again a bit more explicit, which is just great for the intentions of the ZIG programming language.
My only problem is that one thing has not been recognized. I had a kernel_entry_point: *u64 variable, which had been passed directly into a function (so the type is still *u64). In that function, the kernel_entry_point has been set (kernel_entry_point_arg.* = some_value), but the ZIG compiler did not recognize that.
As a workaround, I made the type of the kernel_entry_point to u64 and pointed to that with &kernel_entry_point in the function.
Is this behavior intended?

2 Likes
56

Yes, this sounds correct. From your description I’d guess you were mutating the value pointed to (the u64) and not the value of the pointer itself (.i.e making it point to some other u64 with an assignment). So the pointer should have been const, which is what the compiler was trying to tell you.

4 Likes
57

I agree that if you check in code like that, it’s going to be an improvement to have it complain about unmutated (or even unused) local variables. Marking them right inline in the code as either const instead of var or discarding/mutating its value in a roundabout way to “trick” the compiler into not emitting that error communicates to other readers (myself 3 weeks from now included) more precisely what that variable is intended to be used for. It’s great that I can rely on the compiler to gain confidence in understanding what the code is doing.

But that is not the case I have in mind for allowing unmutated (or unused) local variables. Others in this thread have already pointed this out before, it’s basically about the space between check-ins to version control, or rather the space until a certain set of commits are checked back into mainline (e.g. feature branch merged to main). A transient state of the code, if you will.

Say I’m trying to expand an existing feature in a larger code base, there will already be a lot of friction because the current code base assumes that feature to work a certain way. Since it’s a large code base, and the feature might have many aspects to it that I don’t fully understand yet, I may need to add in a lot of temporary “scaffolding” that I never intend to check into version control. In my experience, this is a good way to explore the unknown corners of the code and gain enough understanding to improve the system and extend it. It is not helpful for the compiler to add even more friction in this specific case. Instead, it would be nice if my trusty compiler could even help me building that temporary “scaffolding” and, at the same time, also help me tear down that temporary “scaffolding” later once it’s no longer required.

Now, I understand that zig’s goal includes “favor reading code over writing code”, which I completely respect and am fully on board with. But there’s also the part in the zig zen about “edge cases matter” and to fully support edge cases in larger systems, I need good ways to explore the code. If it takes a lot of time and effort to fight the compiler to get to a point where I can start understanding the code, I may lose time and energy that I could’ve use to add actual improvements to code.

But if they compiler stays strict, I’m completely fine with it as well. It’s a trade-off. There’s no silver bullet.

I am also completely fine with pushing these kinds of use cases to other tooling, e.g. zls, which already helps a lot with regards to code exploration since it can automatically fix the code for you (adding/removing discards as needed). In a sense, using zls with those features enables, it kind of already is “sloppy mode”, outside of the zig compiler itself.

(Sorry for the wall of text)

3 Likes
58

Trying to compile dependencies (in which I have no control of) with this new restriction is certainly being quite the experience.

6 Likes
59

Consider the 43rd exercise of ziglings. The code doesn’t compile even though i am clearly mutating the struct in printCharacter. Did i miss something?

image
image1920×1080 230 KB

60

This is a case of zig reporting the const variable error too early. There is actually another problem that happens here, but compilation doesn’t report it because it stopped after finding that “local variable is never mutated” error. This is a bit of design flaw, has anyone made a proposal on github for that yet by the way?

Now I don’t want to spoil your experience with ziglings, so let me just reconstruct the actual compile error that you can work with:

test.zig:71:20: error: expected type '*test.Character', found 'test.Character'
    printCharacter(glorp);
                   ^~~~~
test.zig:44:19: note: struct declared here
const Character = struct {
                  ^~~~~~
test.zig:75:22: note: parameter type declared here
fn printCharacter(c: *Character) void {
                     ^~~~~~~~~~
6 Likes