Skip to content

Conversation

@mlugg
Copy link
Member

@mlugg mlugg commented Jan 16, 2025

The main goal of this branch was to make it easier to modify std.builtin by loosening how the compiler converts between comptime-known interned values, and actual std.builtin.Xyz values known to the compiler. However, a zig1.wasm update is desirable for those changes, so I've bundled it together with #22496 to minimize zig1.wasm updates.

For instance, this PR makes it trivial to add/remove fields to/from std.builtin.CallingConvention (@alexrp), or to potentially remove std.builtin.AtomicOrder.unordered (@Rexicon226).

I've written up some guidance on how to do these kinds of std.builtin updates, which I'll put on the wiki shortly.

Supercedes #22496.

This matches established naming conventions. Now is an opportune time to
make this change, since we're already performing breaking changes to
`std.builtin.Type`.
@mlugg mlugg force-pushed the easier-modify-builtin branch from 7a0804c to 1d886f9 Compare January 16, 2025 12:02
@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 Jan 16, 2025
@mlugg
Copy link
Member Author

mlugg commented Jan 16, 2025

https://github.com/ziglang/zig/wiki/How-and-When-to-Update-%60zig1.wasm%60 provides guidance on whether to and how to update zig1.wasm in various cases. Notably, in the case where you're just adding/removing enum/union fields, it's completely unnecessary (the page covers this).

@alexrp
Copy link
Member

alexrp commented Jan 16, 2025

I think this can be flagged as fixing #21842.

mlugg added 6 commits January 16, 2025 12:46
This was done by regex substitution with `sed`. I then manually went
over the entire diff and fixed any incorrect changes.

This diff also changes a lot of `callconv(.C)` to `callconv(.c)`, since
my regex happened to also trigger here. I opted to leave these changes
in, since they *are* a correct migration, even if they're not the one I
was trying to do!
Documentation for this will be on the wiki shortly.

Resolves: ziglang#21842
The commit 2 after this will explain this diff.
Implementing the changes from the prior commit, to prepare for the
following commit.

This also means that zig1 now uses the new value interpret mode, so
that adding and removing fields from `std.builtin` types is easier.

Signed-off-by: mlugg <[email protected]>
For representing struct field default values and array/pointer type
sentinel values, we use `*const anyopaque`, since there is no way for
`std.builtin.Type.StructField` etc to refer back to its `type` field.
However, when introspecting a type, this is quite awkward due to the
pointer casts necessary.

As such, this commit renames the `sentinel` fields to `sentinel_ptr`,
and the `default_value` field to `default_value_ptr`, and introduces
helper methods `sentinel()` and `defaultValue()` to load the values.
These methods are marked as `inline` because their return value, which
is always comptime-known, is very often required at comptime by use
sites, so this avoids having to annotate such calls with `comptime`.

This is a breaking change, although note that 0.14.0 is already a
breaking release for all users of `std.builtin.Type` due to the union
fields being renamed.
@mlugg mlugg force-pushed the easier-modify-builtin branch from 1d886f9 to 9804cc8 Compare January 16, 2025 12:50
@mlugg
Copy link
Member Author

mlugg commented Jan 16, 2025

I think this can be flagged as fixing #21842.

Good point, flagged on the relevant commit.

@mlugg
Copy link
Member Author

mlugg commented Jan 16, 2025

@andrewrk just pinging you to note the existence of the above wiki page, in case you ever find yourself needing to update zig1 for an awkward change.

@mlugg mlugg merged commit 4d8c24c into ziglang:master Jan 16, 2025
10 checks passed
@andrewrk
Copy link
Member

Thanks for typing up the wiki page, that's some really nice documentation 👍

@mlugg
Copy link
Member Author

mlugg commented Jan 16, 2025

Release Notes

These should be placed directly after the notes for #21225.

std.builtin.Type.Pointer.Size Field Renamed

The fields of the std.builtin.Type.Pointer.Size enum have been renamed to lowercase, just like the fields of std.builtin.Type. Again, this is a breaking change, but one which is very easily updated to:

test "pointer type info" {
    comptime assert(@typeInfo(*u8).pointer.size == .One);
}
test "reify pointer" {
    comptime assert(@Type(.{ .pointer = .{
        .size = .One,
        .is_const = false,
        .is_volatile = false,
        .alignment = 0,
        .address_space = .generic,
        .child = u8,
        .is_allowzero = false,
        .sentinel_ptr = null,
    } }) == *u8);
}
const assert = @import("std").debug.assert;

-->

test "pointer type info" {
    comptime assert(@typeInfo(*u8).pointer.size == .one);
}
test "reify pointer" {
    comptime assert(@Type(.{ .pointer = .{
        .size = .one,
        .is_const = false,
        .is_volatile = false,
        .alignment = 0,
        .address_space = .generic,
        .child = u8,
        .is_allowzero = false,
        .sentinel_ptr = null,
    } }) == *u8);
}
const assert = @import("std").debug.assert;

Simplify Usage Of ?*const anyopaque In std.builtin.Type

The default_value field on std.builtin.Type.StructField, and the sentinel fields on std.builtin.Type.Array and std.builtin.Type.Pointer, have to use ?*const anyopaque, because Zig does not provide a way for the struct's type to depend on a field's value. This isn't a problem; however, it isn't particularly ergonomic at times.

Zig 0.14.0 renames these fields to default_value_ptr and sentinel_ptr respectively, and adds helper methods defaultValue() and sentinel() to load the value with the correct type as an optional.

test "get pointer sentinel" {
    const T = [:0]const u8;
    const ptr = @typeInfo(T).pointer;
    const s = @as(*const ptr.child, @ptrCast(@alignCast(ptr.sentinel.?))).*;
    comptime assert(s == 0);
}
test "reify array" {
    comptime assert(@Type(.{ .array = .{ .len = 1, .child = u8, .sentinel = null } }) == [1]u8);
    comptime assert(@Type(.{ .array = .{ .len = 1, .child = u8, .sentinel = &@as(u8, 0) } }) == [1:0]u8);
}
const assert = @import("std").debug.assert;

-->

test "get pointer sentinel" {
    const T = [:0]const u8;
    const ptr = @typeInfo(T).pointer;
    const s = ptr.sentinel().?;
    comptime assert(s == 0);
}
test "reify array" {
    comptime assert(@Type(.{ .array = .{ .len = 1, .child = u8, .sentinel_ptr = null } }) == [1]u8);
    comptime assert(@Type(.{ .array = .{ .len = 1, .child = u8, .sentinel_ptr = &@as(u8, 0) } }) == [1:0]u8);
}
const assert = @import("std").debug.assert;

linusg added a commit to linusg/any-pointer that referenced this pull request Jan 17, 2025
linusg added a commit to linusg/zig-args that referenced this pull request Jan 17, 2025
Spadi0 added a commit to Spadi0/zigimg that referenced this pull request Jan 18, 2025
Fixes builds using Zig master after ziglang/zig#22505 by using the
updated field names in `std.builtin.Type`.
Spadi0 added a commit to Spadi0/zigimg that referenced this pull request Jan 18, 2025
Fixes builds using Zig master after ziglang/zig#22505 was merged by
using the updated field names in `std.builtin.Type`.
scheibo added a commit to pkmn/engine that referenced this pull request Jan 21, 2025
@jiacai2050
Copy link
Contributor

because Zig does not provide a way for the struct's type to depend on a field's value.

Hi, how should I interpret this sentence?

@mlugg mlugg deleted the easier-modify-builtin branch May 18, 2025 20:00
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.

4 participants