ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.37k stars 2.51k forks source link

`self` usage in the std #17105

Open Arnau478 opened 1 year ago

Arnau478 commented 1 year ago

There appears to be two ways of accomplishing the same thing, one (which seems to be more common) is using self:

const Foo = struct {
    fn doSomething(self: *Foo) void {
        //...
    }
};

And the other one, using a specific name, in this case foo:

const Foo = struct {
    fn doSomething(foo: *Foo) void {
        //...
    }
};

Both conventions are used in std (sometimes even in the same file). I think we should settle on a preferred one and update std to follow that. I know this would be a lot of work, but I'm willing to do it if this issue gets accepted.

I personally think we should go with self as:

Examples from the std

Using self

https://github.com/ziglang/zig/blob/d2014fe9713794f6cc0830301a1110d5e92d0ff0/lib/std/Build/Step/Compile.zig#L1269 https://github.com/ziglang/zig/blob/d2014fe9713794f6cc0830301a1110d5e92d0ff0/lib/std/array_list.zig#L157

Using other names

https://github.com/ziglang/zig/blob/d2014fe9713794f6cc0830301a1110d5e92d0ff0/lib/std/Build/Step/Compile.zig#L1067 https://github.com/ziglang/zig/blob/d2014fe9713794f6cc0830301a1110d5e92d0ff0/lib/std/http/Client.zig#L183

nektro commented 1 year ago

going with a non-self name makes the code more portable and most new code does it that way in the stdlib. the one case where folks seem to generally be in consensus on self is in generic types where you have a const Self = @This()

Arnau478 commented 1 year ago

going with a non-self name makes the code more portable

Sorry, why is that? I don't understand the advantage or how that would make it more portable

rohlem commented 1 year ago

I think I remember andrewrk once stating that he dislikes naming the first argument self. (That said, I imagine these opinions can change depending with experience and depending on context(s).)

I don't understand the advantage or how that would make it more portable

I think this argument is rooted in restructuring and moving code between functions. Example (all pseudo-code):

// version 1: inline somewhere
  var array_list = ...;
  array_list.assertUnusedCapacity(n);
  array_list.insertN(item, n);

// version 2: we move it to a dedicated method
fn insertAssumeCapacity(array_list: *Self, n: usize, item: T) void { 
  array_list.assertUnusedCapacity(n); //unchanged from call site
  array_list.insertN(item, n); //unchanged from call site
}

If we change the variable name to self, we have to touch those lines of code. And going in the other direction, if the name was self but you inline/copy the code somewhere else, it's no longer clear what self used to be (versus a self that might be in the destination scope where the code was copied to). At least that's what I think the argument is.

What I do in my personal code is use a more descriptive name than plain Self/self, f.e. HandleSelf, ListSelf, RangeSelf, ConfigSelf etc. . That helps avoiding name collisions of plain Self declarations in nested scopes. Similarly naming the argument list_self, range_self etc. gives the reader a better idea what concept the self in question is.

Vexu commented 1 year ago

I've been thinking of making the same proposal so I'm in favor.

Arnau478 commented 1 year ago

I think this argument is rooted in restructuring and moving code between functions. Example (all pseudo-code):

// version 1: inline somewhere
  var array_list = ...;
  array_list.assertUnusedCapacity(n);
  array_list.insertN(item, n);

// version 2: we move it to a dedicated method
fn insertAssumeCapacity(array_list: *Self, n: usize, item: T) void { 
  array_list.assertUnusedCapacity(n); //unchanged from call site
  array_list.insertN(item, n); //unchanged from call site
}

Oh okay, that makes sense... However, I think it forces you to rethink that bit of functionality. If you're moving it you will most likely have to change something (e.g. make it a pointer). Also, that only happens when the parameter name (array_list in your case) is the same as in the "inline" version, and vice versa.

Similarly naming the argument list_self, range_self etc. gives the reader a better idea what concept the self in question is.

I think we should either call it self or a specific name, but never foo_self. It has de disadvantages of both sides: it forces you to change code both when inlining and when renaming the parent struct.

Arnau478 commented 1 year ago

@He-Pin I think you didn't understand this issue... This is not about any new grammar, it's about std code refactoring. And that piece of code you propose is not valid Zig. If you wanted to propose a grammar change, you can create an issue for that.

He-Pin commented 1 year ago

@He-Pin I think you didn't understand this issue... This is not about any new grammar, it's about std code refactoring. And that piece of code you propose is not valid Zig. If you wanted to propose a grammar change, you can create an issue for that.

Sorry for that, still new, but for this issue, I think it would be better go with the 'self' one which better.