std: remove meta.trait - #18061
std: remove meta.trait#18061andrewrk merged 1 commit into
Conversation
Sorry, something went wrong.
|
std.meta.trait.isZigString should be moved into the main std.meta namespace then, as this is an instrumentally useful function |
Sorry, something went wrong.
There was a problem hiding this comment.
I think there are a few places where old checks should be rewritten rather than removed.
In addition to those below, it might also be worth rewriting the checks in the *As*ReturnType functions in mem.zig, particularly for asBytes/AsBytesReturnType which don't do any field accesses that might help catch incorrect usage, but I'm less sure these cases.
Sorry, something went wrong.
| fn validateType(comptime T: type) void { | ||
| comptime { | ||
| if (!((std.meta.trait.isSlice(T) or | ||
| std.meta.trait.is(.Array)(T) or | ||
| std.meta.trait.isPtrTo(.Array)(T)) and | ||
| std.meta.Elem(T) == u8)) | ||
| { | ||
| @compileError("expect a slice, array or pointer to array of u8, got " ++ @typeName(T)); | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I think it's worth rewriting this check in order to make the error message more helpful. For this test:
const std = @import("std");
test {
const data = [_]u32{1, 2, 3, 4, 5, 6};
_ = std.hash.XxHash64.hash(0, data);
}the current compilation error without validateType is:
lib/std/hash/xxhash.zig:94:45: error: expected type '*const [4]u8', found '*const [4]u32'
acc = finalize4(acc, partial[0..4]);
~~~~~~~^~~~~~
lib/std/hash/xxhash.zig:94:45: note: pointer type child 'u32' cannot cast into pointer type child '[4]u8'
lib/std/hash/xxhash.zig:150:33: note: parameter type declared here
fn finalize4(v: u64, bytes: *const [4]u8) u64 {
^~~~~~~~~~~~
which I think is a little opaque.
Sorry, something went wrong.
| if (bits == 0) return; | ||
|
|
||
| const U = @TypeOf(value); | ||
| comptime assert(trait.isUnsignedInt(U)); |
There was a problem hiding this comment.
I think this should be replaced with comptime assert(@typeInfo(U).Int.signedness == .unsigned);. If an unsigned int is passed in without this check there is checked illegal behaviour (i.e. runtime panic in safe modes) at the @intCasts below.
Sorry, something went wrong.
There was a problem hiding this comment.
I see, thank you
Sorry, something went wrong.
| @@ -315,8 +311,6 @@ pub const XxHash32 = struct { | |||
| } | |||
|
|
|||
| pub fn update(self: *XxHash32, input: []const u8) void { | |||
There was a problem hiding this comment.
Looks like I missed changing this input parameter to anytype if you want to include that here, if not I'll make a PR for it.
Sorry, something went wrong.
What is this useful for? I don't mean that in a combative tone, this is a genuine question since I've never had a use for it. |
Sorry, something went wrong.
|
when you're accepting a value in from an anytype and want to know if it is in the general shape of a []const u8. isZigString handles checking for slice, nul-terminated pointer, pointer-to-array, etc |
Sorry, something went wrong.
|
if youre allowing a function to accept both slices/array pointers and null terminated many pointers then you have to handle them differently regardless in which case using a trait function would be redundant, no? |
Sorry, something went wrong.
|
looks like we have about 70 usages of std.meta.trait. in Bun's codebase 
|
Sorry, something went wrong.
|
It's pretty straightforward to copy those trait functions into the Bun codebase if you want to keep using them. Looking at the screenshot, I would consider every one of those a code smell. |
Sorry, something went wrong.
|
Buckle up, I have even more breakages planned. |
Sorry, something went wrong.
In general, I don't like the idea of std.meta.trait, and so I am providing some guidance by deleting the entire namespace from the standard library and compiler codebase. My main criticism is that it's overcomplicated machinery that bloats compile times and is ultimately unnecessary given the existence of Zig's strong type system and reference traces. Users who want this can create a third party package that provides this functionality. closes #18051
I decided to create a third party package that would retain this functionality and add new functionality depending on user needs in the future. https://github.com/wrongnull/zigtrait |
Sorry, something went wrong.
In particular, the removal of openIterableDir: 519ba9bb654a4d5caf7440160f018b7a3ae1e95a and the removal of std.meta.trait: ziglang/zig#18061
In general, I don't like the idea of std.meta.trait, and so I am providing some guidance by deleting the entire namespace from the standard library and compiler codebase.
My main criticism is that it's overcomplicated machinery that bloats compile times and is ultimately unnecessary given the existence of Zig's strong type system and reference traces.
Users who want this can create a third party package that provides this functionality.
closes #18051