Proposal to catch a class of bugs by adjusting the reporting of discarded locals and parameters

[cryptocode]

Zig Version

0.12.0-dev.161+6a5463951

Steps to Reproduce and Observed Output

Currently, errors related to unused parameters and locals are silenced using _ = <identifier>;

If such a discard exists, and the identifier is used anywhere in the function, you get the following error:

error: pointless discard of function parameter

See #9238 for some reflections on current behavior.

Motivated by an actual bug hunt, I propose that discards are changed to mean "the local variable or parameter can NOT be used after this discard, but it CAN be used before this discard."

I was made aware of these rejected proposals:

#9792
#594

However, those predate discards/pointless discards, and this proposal doesn't include any new keywords - rather it's an adjustment to how discards work.

In addition to the semantic change, the error wording is changed to:

lib.zig:10:9: error: use of discarded local variable
     _ = myvar;
         ^

The discard is no longer considered pointless: it's there to explicitly end the usable scope of myvar. You now get to decide if this is indicative of a bug, or if the discard is no longer valid.

If the discard is at the top of the function, as is common today, then it's equivalent to a pointless discard if the identifier is used (just with different wording)

Below is a typical example of the class of bugs this can prevent, where you use a parameter to compute something that's supposed to be used for the rest of the function. Using the parameter after this point is a bug.

Here we express that "we've computed effective bounds, and you must use this henceforth"

pub fn renderWords(
    self: *@This(),
    input: []const u8,
    bounds: ?Rect,
    horizontal_overflow: HorizontalOverflowBehavior,
    vertical_overflow: VerticalOverflowBehavior,
    horizontal_grow_behavior: GrowBehavior,
    vertical_grow_behavior: GrowBehavior,
) !Point {
    var growth = Point.abs(0, 0);
    var effective_bounds: Rect = if (bounds) |b| b else Rect.init(1, 1, self.width, self.height);
    _ = bounds;

    if (horizontal_grow_behavior == .grow) {

	// ... some complicated rendering logic goes here ...

    	self.moveCursor(.{ .rect = bounds });
    }
}

As expressed in the example, bounds can be used before the discard, but not after.

Then down the line, someone tries to add the moveCursor statement, and, voilà, a potentially subtle and nasty bug is caught:

error: use of discarded function parameter "bounds"

whereas status quo doesn't allow such guards to be set up.

With this, discards become a form of comptime assertion about scope.

Expected Output

error: use of discarded <local variable or parameter>

[Jarred-Sumner]

this idea is great imo

use after free (or after expected lifetime) is too easy in Zig

In Bun’s codebase, we frequently use named scopes with a brk: to ensure the lifetime of a declaration only exists for a certain scope. But we usually only want to limit one variable. Not everything defined in that scope. Using var sometimes has performance implications so we try to avoid that unless necessary

[BratishkaErik]

I don't know if it was brought to the attention earlier, but would it complicate autofix a lot?

[cryptocode]

I don't know if it was brought to the attention earlier, but would it complicate autofix a lot?

It seems doable, though the lsp/zls would probably only add discards and not remove them since discards would no longer be considered pointless.

It did come up in Discord and the unused-autofix seemed less important at least to some. I personally find the autofix for unused so annoying I turn it off, as the unused status is often temporary as you edit - and my editor setup makes saving go into normal-mode+jump point which means I save a lot. Autofix then makes the file unsaved again, so I have to save one more time...

I think catching bugs takes precedence, but if autofix is important to people it obviously needs to be taken into consideration.

[Vexu]

Previously rejected in #9792 and #594

[cryptocode]

Previously rejected in #9792 and #594

These are both mentioned in the issue above, and they're a bit different as they predate discards and thus introduced keywords/intrinsics/potential complexity, which was part of the rejection IIUC. This is a proposal to adjust the behavior of discards, a feature which now already exists.