compile errors for unused things

[andrewrk]

• compile error for unused non-pub global variable
• compile error for unused local variable
• compile error for unused non-pub function decl
• compile error for unreachable code (astgen only)
• even unused pub functions are an error if they are not accessible from outside the package

Note that because of conditional compilation, some things may seem unused when they are in fact used but only depending on some compile variables. Some thought will have to be put into how to make this work without sacrificing compile speed or lazy evaluation.

[ghost]

Would you be able to easily disable these errors? They could get annoying when you're debugging something by e.g. commenting out sections of code. (How does Golang deal with this?)

[andrewrk]

You could disable an error by doing _ = the_unused_variable.

[ghost]

On a related note, the "unreachable code" error is already annoying sometimes, albeit far less annoying than "unused import" errors would be. Adding a "return" statement halfway up a function for testing purposes is something I (try to) do often. I really wish there was an option to turn these into warnings (as an opt-in compiler flag).

Some Go developers say[1] they "get used to" the errors after a while, but there must be a better way (not involving an IDE plugin)? Adding underscores really slows me down when I'm trying to troubleshoot something, constantly commenting/uncommenting and recompiling, and I have to scroll to the top of the file and add/remove underscores from imports every time as well. I'm not always the most patient when trying to learn a new language at home and troubleshooting problems. And then it certainly doesn't help my mood when I search for the problem online and get official FAQs[2] lecturing me not to leave code quality for later and so on.

[1] https://groups.google.com/forum/#!topic/golang-nuts/OBsCksYHPG4
[2] https://golang.org/doc/faq#unused_variables_and_imports

[ghost]

Note that because of conditional compilation, some things may seem unused when they are in fact used but only depending on some compile variables. Some thought will have to be put into how to make this work without sacrificing compile speed or lazy evaluation.

But here comes the tricky part (as if it's not tricky enough already)! Hot code swapping #68. If implemented, hot code swapping will the favorite feature among those that want to make quick experimental changes and see the results immediately.

For example:

fn gameLoop() void {
    normalStuff();
    crazyStuff();
}

The function crazyStuff has been created to contain all of the code additions that you want to experimentally turn on and off while the game is running. If every time you commented out crazyStuff you had to comment out not only crazyStuff itself but all of the functions that only it calls then the speed advantaged and comfort of hot reload is substantially reduced.

With hot code swapping do we even know which functions might get called?

Edit: Would a comptime { _ = crazyStuff; } block solve this? Is it that simple?

[Snektron]

In my opinion the main problem is that this is caught earlier than real compile errors, and that is what hinders (at least my) development workflow. Unused variables, and any of the other errors proposed in this issue / errors which do not affect the ambiguity of the source code, should not stop the compiler from moving on to the semantic analysis phase and generating additional errors. That would at least partially reduce the issue.

IMO thats a prerequisite before implementing any of the other errors.

[buzmeg]

I don't mean to be inflammatory but I really don't think the workflow argument is anything but hypocritical. The commentary of forcing people's hand is interesting, I guess you should definitely fork the project. There's nothing that hasn't been said.

The fork exists. It's called "unzig".

Repo here: https://github.com/markisus/unzig
Patch here: markisus@86b1435

I'm sure he would welcome patches for 0.11 as bootstrapping it from 0.10 is kind of a PITA.

[MCRusher]

This is years old but still ongoing and still being stubbornly opposed for some reason, so I'll leave my two cents anyways:

I've followed this language since when using loops with incorrect syntax would segfault the compiler, several years ago at least.

Documentation, an unstable api, and no package manager has always been an issue, but the singular absolutely massive reason I can't get into using this language even though I'd absolutely love to, is decisions like this made for no objective gain and to the detriment of many users just to suit a small number of people's whims.

No, 'suit' isn't accurate, I meant to say ENFORCE, preferring one way would be fine as long as people could opt-out.

Things like not having multiline comments to comment out blocks of code and expecting people to either comment every individual line out with a single line comment or to use a 3rd party tool

Or not being able to use tabs in code for a very long time, although this seems to have changed since I last checked

And now severely slowing down rapid inline testing and debugging of code by causing compile errors, not warnings, for something as common as a temporarily-unused variable, and future plans to go even further judging by the task list.

I have never, ever in any language had a bug resulting from an unused variable, and even if I had, a warning would be enough to solve it.

These are things no other language I can think of does/did without at least a way to toggle them, and for good reason.

I want to love this language, and there's a whole lot to love, Zig was the betterC alternative that I'd been looking for since years ago, but some things are just a dealbreaker for a language I'd be using because I want to, not because I need to.

[ghost]

The strict behavior of not allowing unused variables does have its merits, but there's been enough comments here explaining why an escape hatch is required (debugging, experimentation).

I suggest that the escape hatch takes the form of a builtin @experimentalDraftCode(), that can have more or less the following behavior:

• In files where this builtin is present, unused variables do NOT result in compile errors.

• Attempts at using (or preferably when creating) a zig package that has code where this builtin is present, DOES result in a compile error

This would hopefully give the best of both worlds. Don't get in the way of experimentation, but also enforce higher quality in code that is meant for reuse.

[Katipo007]

I suggest that the escape hatch takes the form of a builtin @experimentalDraftCode(), that can have more or less the following behavior:

• In files where this builtin is present, unused variables do NOT result in compile errors.

If a built-in approach is decided on, I would personally prefer it to be:

1. Per scope, not per file. This lets you choose to have an entire file/struct in this "experimental" state, or only specific functions.
2. Named something more specific to the behaviour, such as @permitUnused(). Which in my opinion is clearly communicating what the built-in is doing, and is probably more memorable. This would also allow it to be used if/when unused functions also becomes a compile error.

• Attempts at using (or preferably when creating) a zig package that has code where this builtin is present, DOES result in a compile error

I'm unsure about this approach. One of my current projects is a networked multiplayer game, and I have the client, server and common logic split into separate modules/packages (This setup is so that client code cannot access server specific logic, which is meant to be secret and thus prevents any of that secret logic accidentally or purposefully being compiled into player distributed executables). If this couldn't be used across packages/modules, then this feature would be useless for this use-case while prototyping/debugging features that includes client <-> server communication.

My industry experience is predominantly in C++ gamedev (my company is also particularly fast paced). In my time I have seen unused variables be the cause for several bugs, and so I am definitely for unused variables being an error (and I'm working our C++ codebases towards this). But experimention, prototyping & debugging is incredibly valuable to us, and temporarily unused variables are part of that.
I personally dislike the _ = foo; approach for facilitating this experimentation, as it makes these "temporarily unused"s difficult to pick out as part of commit hooks or searching. In my C++ projects I have WARNINGS_PUSH(), WARNINGS_DISABLE_UNUSED() and WARNINGS_POP() macros to wrap experimental scopes, which is much more filterable and recognisable.

[silversquirl]

I personally dislike the _ = foo; approach for facilitating this experimention, as it makes these "temporarily unused"s difficult to pick out as part of commit hooks or searching.

I disagree. I solely use _ = foo for temporarily unused variables, so grep -RE '^\s*_ =' would find all such cases very easily.

For purposefully unused variables (eg. function arguments that are only there in order to fulfill some interface), I instead simply name them _

EDIT: silly me, I forgot about discarding function return values. A regex like ^\s*_ = [^(]+;$ should work reasonably well

[aggregate1166877]

I forgot about discarding function return values.

So by your own admission, your suggested alternative can fail with edge cases. Please don't misunderstand, I'm not trying to be snarky; most people would have silently edited the message and not admitted to the error. I very much appreciate your honesty. But this is a fact: Trying to use outside methods is a trail and error problem, and could lead to missed unused variables, hidden away by an underscore.

[silversquirl]

So by your own admission, your suggested alternative can fail with edge cases

No, I simply forgot a valid usecase for _ = syntax. The updated regex I provided in my edit should catch everything that's not a function call, which includes all cases of temporarily-unused variables.

[arijoon]

I disagree. I solely use _ = foo for temporarily unused variables, so grep -RE '^\s*_ =' would find all such cases very easily.

This is a manual step though. It means the compiler is not even catching the issue it is meant to catch (unused code). You have to manually run a regex over the codebase to delete these. What if another developer doesn't do this? Do you have to add the step of finding all _ = x to the CI? in that case how is this compiler "error" helping in the first place instead of being a compilation flag that you can set for local run and not in the CI.

This single "feature" is the reason I dropped Go shortly after learning about it (its basically a complete deal breaker). It is a shame that some new projects are baking in very opinionated features at a level that is not overridable. My understanding was the aim of Zig is to replace C, this doesn't really help that aim.

[andrewrk]

Prerequisite:

• Formally introduce a class of errors that can be automatically fixed #17584

[silversquirl]

This is a manual step though.

Yes. I was specifically responding to @Katipo007's claim that these cases aren't easily searchable.

The error still helps find unintentional discards, and a linter or similar can find intentional temporary discards, like the ones my regex detects. Yes, it's an extra step to run a linter, but if you remove the error it's also an extra step, and most people run linters as part of their CI already, in pretty much every other language.

[bb010g]

The error still helps find unintentional discards, and a linter or similar can find intentional temporary discards, like the ones my regex detects. Yes, it's an extra step to run a linter, but if you remove the error it's also an extra step, and most people run linters as part of their CI already, in pretty much every other language.

Some compilers also perform linting functionality along with their error checking, and call the reports for these lints "warnings". Zig could offer these lints, without requiring an external linter.

[metroidchild]

@ghost

The strict behavior of not allowing unused variables does have its merits, but there's been enough comments here explaining why an escape hatch is required (debugging, experimentation).

I suggest that the escape hatch takes the form of a builtin @experimentalDraftCode()

@bb010g

Some compilers also perform linting functionality along with their error checking, and call the reports for these lints "warnings". Zig could offer these lints, without requiring an external linter.

Hard agree with both, sometimes code iteration is more important than compiler speed, this is already partially a status quo position with things like instances of structs being able to call static methods, implicit passing of self, implicit types, etc.

Here's my idea for a solution:

• Create an additional optimization mode called something like "DebugWarn".
• This mode adds FMT auto-code-gen as a compilation step, slowdown and all.
• On compilation, notify the user that this this mode is not recommended.
• Whenever FMT finds and auto-fixes an issue on a (temp) file, spit out a warning.

This should obviously not be the default option, but would still give developers the possibility to not get hassled by the compiler at literally every step of the way.

[Raumpatrouille]

I am new to Zig, coming from Rust. There you may name a var _name to show, this var is allowed to be unused.
I often used it when I started a new function and later refurbrided from _name to name.
You easily see it in the code. And a tool easily could check if you still have _anyname in use.

[Pyrolistical]

Should we also add a compile error for "unused mutability"?

const Foo = struct {
    x: u8 = 0,

    // IMO this should compile error, foo.x is not assigned
    pub fn unnecessaryPointer(foo: *Foo) u8 {
        return foo.x;
    }

    pub fn necessaryPointer(foo: *Foo) void {
        foo.x = 42;
    }
};

test {
    const const_foo = Foo{};

    // currently compile errors here
    _ = const_foo.unnecessaryPointer();

    // this correctly compile errors
    const_foo.necessaryPointer();

    var var_foo = Foo{};

    // currently silent
    _ = var_foo.unnecessaryPointer();
}

If foo gets passed to another function that requires a pointer, then mutability is used.

[zoriya]

Pointer does not mean mutability. You can require a pointer for performance concern or to prevent copy of data that is not copy safe.

[Pyrolistical]

Pointer does not mean mutability. You can require a pointer for performance concern or to prevent copy of data that is not copy safe.

Is that not a zig compiler's job to make the tradeoff to pass by reference or pass by copy for const?

[zoriya]

It is but it can get tricky in some cases so its not always applied. You can look at https://www.youtube.com/watch?v=dEIsJPpCZYg for more info.

[silversquirl]

Pointer does not mean mutability

*const T :)

[Pyrolistical]

Should unused @ptrCast error?

test {
    const foo: u8 = 42;
    const bar: *const u8 = &foo;
    const baz = @as(*const u8, @ptrCast(bar));
    _ = baz;
}

[mlugg]

@Pyrolistical, that's not "unused", it's just unnecessary. That is a separate concern.

[mnemnion]

I had a chance to pitch Zig instead of Rust for the low-level component of my project at work and declined to do so specifically because of the inflexibility of the compiler in dealing with non-erroneous behavior. I cannot subject my colleagues to this, especially as the code transitions from greenfield development into something mature that must be supported long-term. Likewise, we work in an environment where much of the code is frequently not included in the release configuration due to performance penalties (yes, we care about 5% performance changes, and diagnostic code can easily add 100% or more to our runtime) and even if this sort of dogmatic approach is actually needed, (and experience with other languages shows that there is no world where it is needed) some of this burden could be trivially removed by allowing an underscore prefix to alert the compiler that, yes, this value bound to a name may or may not be used in a certain configuration, or that the name may be documentation of the meaning of the returned value for those who choose to use it in the future. This is done in other languages and is trivial to implement, and yet here we are due to a baffling intransigence on the part of compiler implementers.

I thought this was worth a substantive reply, which will perhaps help other people who are dissatisfied with Zig's stringency in this matter.

First point is that Zig is an excellent choice for the kind of code you're talking about, where there's a bunch of additional code which shouldn't be included in production builds. Zig has two features here: comptime evaluation, and lazy compilation, which mean that the errors for unused code have zero bearing on the ability to include this sort of code.

You can define a condition in build.zig, like this:

    const options = b.addOptions();
    if (b.option(bool, "diagnostics", "include diagnostic code") orelse false) {
        options.addOption(bool, "diagnostics", true);
    } else {
        options.addOption(bool, "diagnostics", false);
    }

And then use it throughout the codebase, anywhere:

const config = @import("config");

// later:
     const answer = someStruct.askQuestion();
     if  (config.diagnostics {
         someStruct.logABunchOfStuff();
    }

And you can use anonymous structs as namespaces, so this works:

const toolkit = tools: {
    if (config.diagnostics) {
        break :tools struct {
            usingnamespace @include("dianostics/report.zig");
            usingnamespace @include("util/sha3.zig");
        };
    }
};

What's good about this is that toolkit will be void if !config.diagnostics, so if you accidentally use code which isn't intended for production, then it will immediately break the moment you build without the diagnostics flag set.

This generalizes very nicely! For instance, you're writing a function, and you've only used a and b out of a, b, c, and d. You want to test it.

Just do this

const draft = config.draft;

// later 

pub fn somethingWorkedOn(a:T, b:T, c:T, d:T) !void {
    if (draft) {
        _ = c;
        _ = d;
    }
    // rest of function
}

If anything stays unused, you'll eventually set draft to false, and the compiler will helpfully remind you that you didn't use that parameter, so you can remove it. That way, the next person to work with the code (which could be you) isn't going to have to figure out why changing d doesn't do anything, by reading the whole function very carefully just to discover that it isn't used. Then you get to remove it from every callsite! That kind of thing isn't fun.

Now you can rub out lines you aren't using, and you won't get unused variable complaints, it's just:

    if (!draft) thingYouWantNotToHappen(aVariableUsedOnlyHere);

if (!draft) is ten characters, to the realistically three you would use for // . But it's much easier to grep for, and if you adopt a workflow like this, you can make a snippet, surely. I don't think that's heavyweight IDE support, is it? Unless you use nano you have snippets. Make it !d and it's three characters again, including tab-triggering the snippet. Unlike commented out code, this expresses intent, and you can turn it off and on, on a per-build basis. It's a strict improvement!

Zig isn't Go: it has comptime and lazy compilation. If you work with the language, you can avoid the downsides of the compiler being strict about unused things, and still reap the benefits. I don't consider this a workaround for a deficit in the language, and you shouldn't either. Zig provides all the tools you need to make transient changes to your codebase without angering the compiler, and it makes it easy to keep the codebase entirely live, which is a considerable strength.

[ikouchiha47]

Maybe, a macro that tells the compiler or zls not to report errors? Are macros possible in zig? C gives us #define.

[sqrtnull]

After spending half of my time debugging and prototyping fixing these errors, I have decided to stop using the language entirely. Hopefully Andrew reconsiders and allows us to convert these errors into warnings.

Adding code to suppress these errors introduces new problems while only serving to solve a self-inflicted problem. Please reconsider.

[andrewrk]

I am not interested in feedback on this issue.