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

std: update std.builtin.Type fields to follow naming conventions #21225

Merged
merged 2 commits into from
Aug 28, 2024

Conversation

mlugg
Copy link
Member

@mlugg mlugg commented Aug 28, 2024

What can I say? I just felt like breaking everyone's code this release cycle.

This is a prerequisite for a minor language change (disallowing fields and declarations with the same name) which is itself a prerequisite of #9938.

@mlugg mlugg added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. release notes This PR should be mentioned in the release notes. labels Aug 28, 2024
@SeanTheGleaming
Copy link
Contributor

Probably for the best. This had been bugging me forever!

@mlugg mlugg force-pushed the std-builtin-type branch 3 times, most recently from 3200864 to aaa7e73 Compare August 28, 2024 07:39
mlugg added 2 commits August 28, 2024 08:39
The compiler actually doesn't need any functional changes for this: Sema
does reification based on the tag indices of `std.builtin.Type` already!
So, no zig1.wasm update is necessary.

This change is necessary to disallow name clashes between fields and
decls on a type, which is a prerequisite of ziglang#9938.
These won't live in the parent namespace as decls which causes problems
later in this function, and tests are guaranteed not to be referenced at
comptime anyway, so there's actually no need to run this logic.
@mlugg
Copy link
Member Author

mlugg commented Aug 28, 2024

@andrewrk I'm confident that the changes are correct, but idk if you want to look this over anyway. Perhaps just double-check that the field names in Type are what you expected (e.g. I've used .noreturn even though a "direct" rewrite of the old tag name would be .no_return)

@andrewrk andrewrk merged commit 31fef6f into ziglang:master Aug 28, 2024
10 checks passed
@andrewrk
Copy link
Member

Looks good. It makes sense for result of reflection to use the same names as the types.

scheibo added a commit to pkmn/engine that referenced this pull request Aug 29, 2024
rather tedious to have to support both naming styles
linusg added a commit to linusg/zig-args that referenced this pull request Aug 30, 2024
linusg added a commit to linusg/any-pointer that referenced this pull request Aug 30, 2024
linusg added a commit to linusg/zig-args that referenced this pull request Aug 30, 2024
linusg added a commit to linusg/parser-toolkit that referenced this pull request Aug 30, 2024
ikskuh pushed a commit to ikskuh/parser-toolkit that referenced this pull request Aug 30, 2024
ikskuh pushed a commit to ikskuh/zig-args that referenced this pull request Aug 30, 2024
@mlugg
Copy link
Member Author

mlugg commented Nov 11, 2024

Release Notes

std.builtin.Type Fields Renamed

In most cases, Zig's standard library follows the naming conventions given [in the language reference](https://ziglang.org/documentation/0.14.0/#toc-Names\). Zig 0.14.0 updates the fields of the std.builtin.Type tagged union to follow these conventions by lowercasing them:

pub const Type = union(enum) {
    type: void,
    void: void,
    bool: void,
    noreturn: void,
    int: Int,
    float: Float,
    pointer: Pointer,
    array: Array,
    @"struct": Struct,
    comptime_float: void,
    comptime_int: void,
    undefined: void,
    null: void,
    optional: Optional,
    error_union: ErrorUnion,
    error_set: ErrorSet,
    @"enum": Enum,
    @"union": Union,
    @"fn": Fn,
    @"opaque": Opaque,
    frame: Frame,
    @"anyframe": AnyFrame,
    vector: Vector,
    enum_literal: void,
    // ...
};

Note that this requires using "quoted identifier" syntax for @"struct", @"union", @"enum", @"opaque", and @"anyframe", because these identifiers are also keywords.

This change is widely breaking, but upgrading is simple:

test "switch on type info" {
    const x = switch (@typeInfo(u8)) {
        .Int => 0,
        .ComptimeInt => 1,
        .Struct => 2,
        else => 3,
    };
    try std.testing.expect(0, x);
}
test "reify type" {
    const U8 = @Type(.{ .Int = .{
        .signedness = .unsigned,
        .bits = 8,
    } });
    const S = @Type(.{ .Struct = .{
        .layout = .auto,
        .fields = &.{},
        .decls = &.{},
        .is_tuple = false,
    } });
    try std.testing.expect(U8 == u8);
    try std.testing.expect(@typeInfo(S) == .Struct);
}
const std = @import("std");

-->

test "switch on type info" {
    const x = switch (@typeInfo(u8)) {
        .int => 0,
        .comptime_int => 1,
        .@"struct" => 2,
        else => 3,
    };
    try std.testing.expect(0, x);
}
test "reify type" {
    const U8 = @Type(.{ .int = .{
        .signedness = .unsigned,
        .bits = 8,
    } });
    const S = @Type(.{ .@"struct" = .{
        .layout = .auto,
        .fields = &.{},
        .decls = &.{},
        .is_tuple = false,
    } });
    try std.testing.expect(U8 == u8);
    try std.testing.expect(@typeInfo(S) == .@"struct");
}
const std = @import("std");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. release notes This PR should be mentioned in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants