Skip to content

change field names in @typeInfo to be [:0]const u8 (null-terminated) instead of []const u8 #16072

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

Closed
IntegratedQuantum opened this issue Jun 17, 2023 · 7 comments · Fixed by #18470
Labels
accepted This proposal is planned. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@IntegratedQuantum
Copy link
Contributor

Zig Version

0.11.0-dev.3655+a2f54fce5 worked in 0.11.0-dev.3395+1e7dcaa3a

Steps to Reproduce and Observed Behavior

const std = @import("std");
x: void,
pub fn main() !void {
	@compileLog(@typeInfo(@This()).Struct.fields[0].name[0..]);
}

Output:

$ zig run test.zig
Compile Log Output:
@as(*const [1]u8, "x")
$
$ ~/Downloads/zig-old/zig run test.zig
Compile Log Output:
@as(*const [1]u8, "x\x00")

In my project I was using field names to automatically generate calls to OpenGL's glGetUniformLocation.

Expected Behavior

I expect field names to be null-terminated, given that most other compiler-generated strings(string literals, @embedFile, @errorName) are also null-terminated for C compatibility.

@IntegratedQuantum IntegratedQuantum added the bug Observed behavior contradicts documented or intended behavior label Jun 17, 2023
@Vexu Vexu added the regression It worked in a previous version of Zig, but stopped working. label Jun 17, 2023
@Vexu Vexu added this to the 0.11.0 milestone Jun 17, 2023
@Jarred-Sumner
Copy link
Contributor

Jarred-Sumner commented Jun 17, 2023

This is the current blocker for Bun to upgrade Zig

Unrelated: there's an issue with GenericPoison that hadn't happened in zig of a month ago when passing an anytype param to a function pointer when the type is known at comptime

@ghost
Copy link

ghost commented Jun 18, 2023

I think I've found the code responsible for this! I'm going to keep at it for a few more hours and if I don't have a fix tonight I'll post my findings.

@ghost
Copy link

ghost commented Jun 18, 2023

Well I haven't been able to get it to work. I was looking in Sema.zig: zirTypeInfo, under the .Struct section. For a while I tried to change .slice_const_u8_type to .slice_const_u8_sentinel_0_type and add .sentinel = .zero_u8 in the call to mod.arrayType, but this kept crashing on the assert at InternPool.zig:3880, where it says assert(ip.typeOf(elem) == field.ty.toIntern());. I think I found out this was because parts of the compiler use std.builtin.Type.StructField, which says the field name's type is []const u8, and I hadn't updated that definition. But, then I noticed that even in old versions where your code works as expected, std.builtin.Type.StructField still says []const u8.

The last patch I tried was this:

diff --git a/src/Sema.zig b/src/Sema.zig
index 36fe5a6ee..fc8dff605 100644
--- a/src/Sema.zig
+++ b/src/Sema.zig
@@ -17234,9 +17234,9 @@ 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
-                                    try sema.arena.dupe(u8, ip.stringToSlice(ip.indexToKey(struct_ty.toIntern()).anon_struct_type.names[i]))
+                                    try sema.arena.dupeZ(u8, ip.stringToSlice(ip.indexToKey(struct_ty.toIntern()).anon_struct_type.names[i]))
                                 else
-                                    try std.fmt.allocPrint(sema.arena, "{d}", .{i});
+                                    try std.fmt.allocPrintZ(sema.arena, "{d}", .{i});
                                 const new_decl_ty = try mod.arrayType(.{
                                     .len = bytes.len,
                                     .child = .u8_type,
@@ -17290,7 +17290,7 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai
                     struct_obj.fields.values(),
                 ) |*field_val, name_nts, field| {
                     // TODO: write something like getCoercedInts to avoid needing to dupe
-                    const name = try sema.arena.dupe(u8, ip.stringToSlice(name_nts));
+                    const name = try sema.arena.dupeZ(u8, ip.stringToSlice(name_nts));
                     const name_val = v: {
                         var anon_decl = try block.startAnonDecl();
                         defer anon_decl.deinit();

Trying to update the values without updating the types. But this seemed to have no effect.

@DaanVandenBosch
Copy link

I'm passing field names to the win32 GetProcAddress function and plan on doing something similar to load Vulkan symbols. Null-terminated strings are very helpful here.

@andrewrk
Copy link
Member

I'm a bit confused here; the type of field names has always been []const u8, so I'm not sure where y'all think this null terminator was coming from.

name: []const u8,

That said, if we consider this a proposal to change that to be name: [:0]const u8 instead, then I'm in favor of that.

@andrewrk andrewrk added contributor friendly This issue is limited in scope and/or knowledge of Zig internals. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. accepted This proposal is planned. and removed bug Observed behavior contradicts documented or intended behavior regression It worked in a previous version of Zig, but stopped working. labels Jun 20, 2023
@andrewrk andrewrk changed the title Regression: field names from @typeInfo no longer contain null terminator \x00. change field names in @typeInfo to be [:0]const u8 (null-terminated) instead of []const u8 Jun 20, 2023
@Jarred-Sumner
Copy link
Contributor

Jarred-Sumner commented Jun 20, 2023 via email

@Vexu
Copy link
Member

Vexu commented Jun 20, 2023

There is also #9182

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
5 participants