std: remove meta.trait #18061
std: remove meta.trait #18061
Conversation
andrewrk
added
breaking
|
|
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.
| 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.
| @@ -35,7 +33,6 @@ pub fn BitWriter(comptime endian: std.builtin.Endian, comptime WriterType: type) | |||
| 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.
| @@ -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.
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. |
|
when you're accepting a value in from an |
|
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? |
|
looks like we have about 70 usages of 
|
|
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. |
|
I don't disagree but optics-wise the communication could be a little more diplomatic than just "ur bad" when justifying the breaking of production code, as it sets the tone to spectators judging the community. |
|
Buckle up, I have even more breakages planned. |
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
andrewrk
force-pushed
the
remove-std.meta.trait
branch
from
5e48c0a
to
a3f2f60
Compare
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 |
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