zig icon indicating copy to clipboard operation
zig copied to clipboard

only return decls from `@typeInfo` that are visible to the caller

Open nektro opened this issue 2 years ago • 2 comments

Fixes #10731

nektro avatar Mar 11 '23 09:03 nektro

Shouldn't is_pub still exist as you can access "private" declarations from the current file without knowing whether they're public or not? Or is there just no good reason for providing information about a declaration's visibility?

SuperAuguste avatar Mar 11 '23 17:03 SuperAuguste

I can add it back, but do you have a use case where it would be needed to make that disambiguation? there weren't any places where that was being done in zig itself or my local projects during my testing. All of the cases I observed .is_pub was being inspected as a stop gap to this being implemented. Since the non-pub decls you'd be skipping over in an external file had no reason to be inside the container in a local file. Consider the following two examples:

// local.zig

const std = @import("std");

pub const S = struct {
    pub const a = 45;
    pub const b = 87;
    pub const c = 93;
};

comptime {
    @compileLog(@typeInfo(S)); // .Struct.decls will only include a, b, c
    // std is skipped because its outside the container
}
// external.zig

const std = @import("std");

pub const a = 45;
pub const b = 87;
pub const c = 93;
// other.zig

const ex = @import("external.zig");

comptime {
    @compileLog(@typeInfo(ex)); // .Struct.decls will only include a, b, c
    // std was skipped because it was non-pub
}

nektro avatar Mar 11 '23 18:03 nektro

I can add it back, but do you have a use case where it would be needed to make that disambiguation? there weren't any places where that was being done in zig itself or my local projects during my testing. All of the cases I observed .is_pub was being inspected as a stop gap to this being implemented. Since the non-pub decls you'd be skipping over in an external file had no reason to be inside the container in a local file. Consider the following two examples:

// local.zig

const std = @import("std");

pub const S = struct {
    pub const a = 45;
    pub const b = 87;
    pub const c = 93;
};

comptime {
    @compileLog(@typeInfo(S)); // .Struct.decls will only include a, b, c
    // std is skipped because its outside the container
}
// external.zig

const std = @import("std");

pub const a = 45;
pub const b = 87;
pub const c = 93;
// other.zig

const ex = @import("external.zig");

comptime {
    @compileLog(@typeInfo(ex)); // .Struct.decls will only include a, b, c
    // std was skipped because it was non-pub
}

With the current behavior of usingnamespace, this assumption is not necessarily correct. The following file will probably include private decls from std.meta and I believe these can be accessed by code in std.meta.

pub usingnamespace @import("std").meta;
comptime {
    @compileLog(@import("std").meta.declarations(T));
}

I think this usingnamespace behavior is a footgun and should be eliminated: for example, it causes errors if you have decls in multiple usingnamespace files that conflict, even if they are private in their origin files. However, it is an example of why is_pub is necessary as long as status quo usingnamespace behavior is kept.

Luexa avatar Mar 22 '23 11:03 Luexa

Alternatively to my previous comment, you can use a separate file to detect whether it's pub or not, but this seems messy.

meta.zig would be modified to only include public symbols in declaration functions and a convenience method to check if a decl is defined and truly public. The hasPubDecl function in meta.zig would import an implementation from another file since it can't possibly be file-local in multiple files if the other file is never assigned to a constant.

// std/meta/has_pub_decl.zig

/// Internal function used to detect whether a symbol is
/// public everywhere, even in cases where `@hasDecl` would
/// return true in the current scope.
pub fn hasPubDecl(comptime T: type, comptime decl_name: []const u8) bool {
    return @hasDecl(T, decl_name);
}
// std/meta.zig

/// Return whether `T` has a decl named `decl_name` which
/// could can be accessed from any file with access to `T`.
pub fn hasPubDecl(comptime T: type, comptime decl_name: []const u8) bool {
    return comptime @import("meta/has_pub_decl.zig").hasPubDecl(T, decl_name);
}

/// Return a list of globally public declarations in `T`.
/// Prefer obtaining this through `@typeInfo` if local declarations are necessary.
pub fn declarations(comptime T: type) []const std.builtin.Type.Declaration {
    comptime {
        var pub_decls: []const std.builtin.Type.Declaration = &.{};
        switch (@typeInfo(T)) {
            inline .Struct, .Union, .Enum, .Opaque => |info| {
                for (info.decls) |decl| {
                    if (hasPubDecl(T, decl.name)) {
                        pub_decls = pub_decls ++ &[_]std.builtin.Type.Declaration{ decl };
                    }
                }
                if (info.decls.len == pub_decls.len) {
                    return info.decls;
                } else {
                    return pub_decls;
                } 
            },
            else => @compileError("Unsupported type " ++ @typeName(T) == " cannot contain declarations"),
        }
    }
}

Luexa avatar Mar 22 '23 12:03 Luexa

closing abandoned PR. CI checks not passing; conflicts.

andrewrk avatar Jun 18 '23 08:06 andrewrk