Skip to content

Proposal: Make ambiguous field/decl access in the presence of shadowing a compile error #7942

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

Closed
ifreund opened this issue Feb 3, 2021 · 5 comments
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@ifreund
Copy link
Member

ifreund commented Feb 3, 2021

Currently Zig allows decls and fields of the same struct to share names. This is confusing, especially if the field in question is a function pointer:

const std = @import("std");

const Foo = struct {
    bar: fn () void,
    fn bar(foo: Foo) void {
        std.debug.print("decl bar called\n", .{});
    }
};

fn bar2() void {
    std.debug.print("field bar called\n", .{});
}

pub fn main() void {
    const foo = Foo{ .bar = bar2 };
    foo.bar();
}

The current rule is that the field shadows the decl, but this is non-obvious at the call-site if the user intended to call Foo.bar() instead of the function pointer stored in the field bar. It should be noted that this is technically not ambiguous as field always shadows the decl and the decl may be accessed with Foo.bar(&foo);. Nonetheless, I propose to make this a compile error as I don't think the ability to use the same name for fields and decls is a beneficial feature that outweighs the confusion and potential for errors caused by this situation.

See also: #705 #6966

@SpexGuy SpexGuy added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Mar 17, 2021
@andrewrk andrewrk changed the title Proposal: Make field/decl shadowing a compile error Proposal: Make ambiguous field/decl access in the presence of shadowing a compile error May 19, 2021
@andrewrk andrewrk added this to the 0.9.0 milestone May 19, 2021
@andrewrk andrewrk added the accepted This proposal is planned. label May 19, 2021
@andrewrk
Copy link
Member

The plan here is to follow the example set by decl shadowing: fields and declarations are allowed to shadow; however access must always be unambiguous. So the example given above would be a compile error.

@ifreund
Copy link
Member Author

ifreund commented May 19, 2021

access must always be unambiguous. So the example given above would be a compile error.

As I explained above, this is already technically unambiguous if the (subtle) rules are understood. I assume your proposed change is to force field access to look unambiguous as well? @field(foo, bar) is still ambiguous as @field() works for decls as well despite its name (#3839).

@ifreund
Copy link
Member Author

ifreund commented Jul 29, 2021

So I brought this up in the stage2 meeting today and it looks like one option to make this appear less ambiguous would be to implement the following behavior:

foo.bar(); // compile error
(foo.bar)(); // function pointer field call
Foo.bar(foo); // function decl call

I still feel like this isn't perfect though as the function pointer field may be accessed without calling it, and requiring it to be wraped in parentheses in e.g. @TypeOf((foo.bar)) seems nonsensical.

The only way I see to resolve this issue in a way I personally find satisfactory seems to be #6966 or similar.

@ominitay
Copy link
Contributor

ominitay commented Jun 6, 2022

I personally believe that field/decl shadowing should eliminated from the language. I don't see shadowing here being either necessary or useful: it instead introduces ambiguity.

@SeanTheGleaming
Copy link
Contributor

I believe this is resolved as of #21231

@Vexu Vexu closed this as completed Feb 12, 2025
@Vexu Vexu modified the milestones: 0.15.0, 0.14.0 Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. 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

6 participants