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

Allow re-declaring variables #14913

Closed
silversquirl opened this issue Mar 14, 2023 · 9 comments
Closed

Allow re-declaring variables #14913

silversquirl opened this issue Mar 14, 2023 · 9 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.

Comments

@silversquirl
Copy link
Contributor

Zig's shadowing rules (ie. "you can't") are excellent at avoiding bugs and aiding readability, with one exception: re-declaring variables. There are a few situations where you'd want to do this:

  1. exposing a generic argument in a container:
    fn Generic(comptime Thing: type) type {
        return struct {
            pub const Thing = Thing; // Needs to be exposed to other users
        };
    }
  2. making a variable immutable:
    var foo: u32 = 3;
    foo += 7;
    const foo = foo; // After this point, foo is immutable
  3. making a function argument mutable:
    fn foo(x: MyStruct) MyStruct {
        var x = x; // x is now mutable
        x.y += 7;
        return x;
    }

I propose that Zig relaxes its shadowing rules specifically for declarations of the form {const,var} <x> = <x>, ie. redeclaring a variable with no modifications.
This would still prevent bugs where the wrong variable is accidentally modified due to eg. a local being renamed, and would aid readability by avoiding pointless extra names in cases like pub const Thing = ThingArg or var foo = foo_const.
It would also reduce bugs caused by mutating a variable after it shouldn't be modified, which is currently not possible without using blocks (which can be pretty ugly and require extra names - and still doesn't work for cases where two variables need to be mutated at once and then const-ified).

@silversquirl
Copy link
Contributor Author

For some examples of my last point about blocks:

// Currently
const x = a: {
    var x_var: u32 = 3; // needs a different name because shadowing
    x_var += 7;
    break :a x_var;
};

// Proposed
var x: u32 = 3;
x += 7;
const x = x;

// Currently
const xy = a: {
    var x: u32 = 3;
    var y: u32 = 4;
    y += x;
    x += y;
    break :a .{ .x = x, .y = y };  // Have to use a struct here because we can't break with two values
};

// Proposed
var x: u32 = 3;
var y: u32 = 4;
y += x;
x += y;
// No need for a struct because we can just `const`-ify the variables
const x = x;
const y = y;

@mlugg
Copy link
Member

mlugg commented Mar 14, 2023

Case 1 is a good use case, and imo the best justification for this proposal.

Case 2 is a bit problematic because of this:

var x = ...;
const p = &x;
const x = x;

x's container would imply that it can't be mutated after this. However, we don't want this to actually copy x to a new place on the stack (it might be a big structure!) and so we can mutate it through p in practice. This code looks totally valid, and in a larger snippet (where maybe the pointer is stored in something else) it'd be hard to notice the subtly incorrect (or at least unintuitive) behaviour here.

I think blocks are a neater solution to this, and the "multiple variables" issue I hope can instead be resolved through tuple destructuring syntax.

Case 3 is interesting - there's nothing immediately problematic about it as far as I can tell, but personally I quite like having to give the mutable variant a different name to make clear that it's distinct. Not sure how popular that opinion is however.

@scheibo
Copy link
Contributor

scheibo commented Mar 14, 2023

I find Zig's shadowing rules kind of annoying, particularly in case (3), but I'm not sure allowing this form of redeclaration is the best answer because it then violates the very useful property that if you see a name declared const when reading code you know that it can never change. The property makes reasoning about code super useful.

Allowing function parameters to be declared var in the signature would solve the same issue as (3) but without "redeclaring" anything

fn foo(var x: u8, y: bool, var z: u8) {

EDIT: actually I'm not sure how much I like this, as things then get confusing with pointers and Zig's semantics of how it transparently decides whether to pass by value vs. by reference

@silversquirl
Copy link
Contributor Author

I don't agree that it violates that property. A variable declared as const can still never change, but you can introduce a new variable named the same thing that can. It's a slightly pedantic difference, but I think it's an important one.

I don't like var in arguments because it's extra noise in parameters and makes pass-by-reference optimizations more annoying for the compiler (as you mention in your edit). It also makes it easy make parameters mutable, when that's something that should probably be carefully considered - so I prefer the more explicit var x = x syntax.

@scheibo
Copy link
Contributor

scheibo commented Mar 14, 2023

I don't agree that it violates that property. A variable declared as const can still never change, but you can introduce a new variable named the same thing that can. It's a slightly pedantic difference, but I think it's an important one.

Ahaha I literally though to add "(pedantically it is a new variable with the same name)" to my original reply, but I didn't think anyone would actually try to argue that. It is 100% a distinction without a difference IMO, it still absolutely violates the mental model of "when I see const x in a scope I know that in the rest of the scope x never changes"

@andrewrk andrewrk closed this as not planned Won't fix, can't repro, duplicate, stale Mar 14, 2023
@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Mar 14, 2023
@andrewrk
Copy link
Member

duplicate of rejected proposal #594

@mlugg
Copy link
Member

mlugg commented Mar 14, 2023

This doesn't seem like a duplicate of that; this doesn't allow arbitrary redeclaration, but only in specific cases where the actual value of the variable isn't changed. (I'm not sure whether I agree with all of it, but at least case 1 I think might be good.)

@silversquirl
Copy link
Contributor Author

I also don't see how this issue is related to #594, that seems to be proposing something very different.

@andrewrk
Copy link
Member

I acknowledge that this proposal is not a duplicate after all. However it is still rejected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

4 participants