Make @typeInfo return null-terminated strings
#18470
Conversation
Changes the types of `std.builtin.Type` `name` fields from `[]const u8` to `[:0]const u8`, which should make them easier to pass to C APIs expecting null-terminated strings. This will break code that reifies types using `[]const u8` strings, such as code that uses `std.mem.tokenize()` to construct types from strings at comptime. Luckily, the fix is simple: simply concatenate the `[]const u8` string with an empty string literal (`name ++ ""`) to explicitly coerce it to `[:0]const u8`. Co-authored-by: Krzysztof Wolicki <[email protected]>
These changes can be reverted the next time stage1 is updated.
| @@ -17658,22 +17661,23 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai | |||
| // TODO: write something like getCoercedInts to avoid needing to dupe | |||
| const bytes = if (tuple.names.len != 0) | |||
| // https://github.com/ziglang/zig/issues/15709 | |||
There was a problem hiding this comment.
Can this comment be removed? The issue was closed in May last year.
There was a problem hiding this comment.
Yes, it can. But maybe it will be nicer if I just merge your PR for now since it's already green.
|
Nice work, I think I've closed my PR as I was unable to work on fixing it properly at that time |
| @@ -5315,7 +5315,7 @@ pub fn get(ip: *InternPool, gpa: Allocator, key: Key) Allocator.Error!Index { | |||
|
|
|||
| try ip.extra.ensureUnusedCapacity( | |||
| gpa, | |||
| @typeInfo(Tag.Aggregate).Struct.fields.len + @as(usize, @intCast(len_including_sentinel)), | |||
| @typeInfo(Tag.Aggregate).Struct.fields.len + @as(usize, @intCast(len_including_sentinel + 1)), | |||
There was a problem hiding this comment.
The CI run for Linux ran into a buffer overflow panic when initializing a sentinel-terminated array for reasons seemingly unrelated to the @typeInfo changes.
https://github.com/ziglang/zig/actions/runs/7439194713/job/20238792974
I based this fix on
Line 5274 in 804cee3
I'm not at all familiar with compiler internals so I could be completely off base, but from skimming through code from zirArrayInit to here it appears to me that the problem is that aggregate.storage.elems includes the sentinel
const final_len = try sema.usizeCast(block, src, array_ty.arrayLenIncludingSentinel(mod));
const resolved_args = try gpa.alloc(Air.Inst.Ref, final_len);
// ...
const elem_vals = try sema.arena.alloc(InternPool.Index, resolved_args.len);
// ...
const arr_val = try mod.intern(.{ .aggregate = .{
.ty = array_ty.toIntern(),
.storage = .{ .elems = elem_vals },
} });which makes these two operations require an available capacity of len_including_sentinel + 1
ip.extra.appendSliceAssumeCapacity(@ptrCast(aggregate.storage.elems));
if (sentinel != .none) ip.extra.appendAssumeCapacity(@intFromEnum(sentinel));Judging by the related (?) fix in 5fa260b it's possible that elems should be sliced to exclude the sentinel
-ip.extra.appendSliceAssumeCapacity(@ptrCast(aggregate.storage.elems));
+ip.extra.appendSliceAssumeCapacity(@ptrCast(aggregate.storage.elems[0..@intCast(len)]));
if (sentinel != .none) ip.extra.appendAssumeCapacity(@intFromEnum(sentinel));but I have no idea what the consequences of doing so would be so I decided not to touch it.
|
Nice work! |
* `name` field of std.builtin.Type struct changed type from `[]const u8` to `[:0]const u8`: ziglang/zig#18470 . * New `'comptime var' is redundant in comptime scope` error introduced in ziglang/zig#18242 . Signed-off-by: Eric Joldasov <[email protected]>
Closes #16072
Closes #9182
Changes the types of
std.builtin.Typenamefields from[]const u8to[:0]const u8, which should make them easier to pass to C APIs expecting null-terminated strings (e.g. OpenGL'sglGetUniformLocation).This will break code that reifies types using
[]const u8strings, for example code that usesstd.mem.tokenize()to construct types from strings at comptime. Luckily, there is a simple fix: simply concatenate the[]const u8string with an empty string literal (name ++ "") to create a new string of type[:0]const u8. Type reification always happens at comptime so this trick should always possible.The brunt work was based on #16330 by @der-teufel-programming which was closed just as quickly as it was opened for unknown reasons.
I opted to work around stage1 still returning
[]const u8strings in code instead of updating stage1, since it affected very few lines of code actually needed by the compiler. These changes have been isolated to the second commit, which should be able to be reverted after the next time stage1 has been updated.