Finally managed to get this to work, but it feels like inline should not be necessary here: ghext/src/ghext.zig at 111d78439b0d3ce885a5205ace1933e91724334f · charlesrocket/ghext · GitHub
without inline I am getting cast errors or unresolved comptime
src/ghext.zig:216:21: error: expected type '*ghext', found '*const ghext'
const head = ghx.hash(HashLen.Short, Worktree.Unchecked);
~~~^~~~~
src/ghext.zig:216:21: note: cast discards const qualifier
src/ghext.zig:142:19: note: parameter type declared here
pub fn hash(self: *Ghext, comptime length: HashLen, check: Worktree) []const u8 {
^~~~~~
If inline is removed, only one (hash_short) test is passing.
Since the hash function does not modify self you can fix the error by changing pub fn hash(self: *Ghext,... to pub fn hash(self: *const Ghext,...
Nope, it only works with inline. If I drop it and enforce const:
src/ghext.zig:155:17: error: unable to resolve comptime value
head ++ "-unverified"
^~~~
src/ghext.zig:155:17: note: slice being concatenated must be comptime-known
The ++ concatenation operation concatenates arrays. You are attempting to use it to concatenate slices and return a slice, which does not work. Even in the Short case where your slices have comptime length, where will the memory for the concatenated slices live? The only reason it is currently working with inline is because you are calling the function with Unchecked and thus the concatenation portion is not being evaluted. If you call the current inline version with Checked then you get the same compile error.
Checked also works, but only hash_short is passing. ++ / ** works for both slices and arrays.
Added more tests (Checked + Unchecked) and now getting unresolved comptime with inline as well 
I suspect that hash_short is passing because self.head[0..7] coerces to *const [7]u8 so the compiler knows how much memory head ++ "..." will take up at comptime. It can’t know that when the length of self.head is runtime-known.
As permutationlock has already said, you can’t use ++ on a slice with runtime known length because where would you store it? The compiler would have to reserve std.math.maxInt(usize) + "...".len bytes for your concatenation to cover every possible length and that isn’t possible/feasible.
I’m surprised that this code works with inline probably there’s some lazy compilation magic going on there.
Generally you can use std.mem.concat() instead of ++ to make this work, of course this requires an allocator. But since you’re dealing with commit hashes here which have a fixed length you can also just use a std.BoundedArray or something similiar.
self.head is also comptime. But I guess I am introducing a mutation somewhere down the line.
The idea is to pull a commit hash to append to a version string, so everything must be comptime. Ill check the BoundedArray, thanks! Tho, the issue is still quite confusing because it should never work as far as I can tell. But it does, in some cases.
How is self.head comptime? You’re getting it from a child process at runtime or am I missing something?
When I mark it as comptime compiler complains about the redundancy. Maybe I pay too much attention to the compiler complaints. The init function uses a var to return a comptime struct, or at least thats the idea (const head = try arr.toOwnedSlice();). readWithGit() should be done in comptime and it worked so far. Maybe theres something else going on there.
I am surprised that you can concatenate slices with runtime contents and comptime length. You have to be very careful where you use that because it is “silently” using a single location in static memory (or maybe its on the stack and this illegal/undefined behavior?).
const std = @import("std");
fn foo(slice: []const u8) []const u8 {
return slice[0..8] ++ " \\";
}
test {
const str = "hello this is a string";
const foo_res = foo(str);
try std.testing.expectEqualSlices(u8, "hello th \\", foo_res);
const foo_res2 = foo("I hope this doesn't overwrite my first string");
try std.testing.expectEqualSlices(u8, "I hope t \\", foo_res2, );
try std.testing.expectEqualSlices(u8, "hello th \\", foo_res);
}
Yea I am kinda lost too) For some reason everything worked like a clock until I decided to handle both long and short hashes via hash().
Where exactly does the compiler complain about that? As soon as you use an allocator a value cannot be comptime anymore (unless your allocator works at comptime as well which std.testing.allocator does not) because the memory allocated by the allocator comes from the OS at runtime and cannot be known to the compiler. As far as I’m aware Zig also can’t execute child processes at comptime.
Try explicitly forcing comptime with var ghx = comptime try Ghext.init(std.testing.allocator); it won’t compile.
Yeah I think this returns a dangling stack pointer. String literals are usually stored in .rodata AFAIK and you can’t exactly do that with a runtime-known slice/array pointer. Although in your example this might actually be the case because all values are comptime-known.
EDIT: just confirming this:
const literal = "asdhfjklsadfhsdjklafhsdajfklsahdfljksadhfkhaksjr"; // 48 chars
export fn concat(slc: *const [8]u8) *const [8 + literal.len]u8 {
return slc ++ literal;
}
export fn use(slc: *const [8]u8) u32 {
const str = slc ++ literal;
var result: u32 = 0;
for (0..str.len - 8) |i| {
for (str[i..][0..8]) |c| result += c;
}
return result;
}
With -O ReleaseFast -mcpu x86_64:
concat:
// doesn't even perform concatenation, just returns stack pointer
push rbp
mov rbp, rsp
lea rax, [rbp - 57]
pop rbp
ret
use:
push rbp
// copy slc param to stack[-56..-48]
mov rbp, rsp
mov rax, qword ptr [rdi]
mov qword ptr [rbp - 56], rax
// copy literal to stack[-48..0]
movups xmm0, xmmword ptr [rip + __anon_194]
movups xmmword ptr [rbp - 48], xmm0
movups xmm0, xmmword ptr [rip + __anon_194+16]
movups xmmword ptr [rbp - 32], xmm0
movdqu xmm0, xmmword ptr [rip + __anon_194+31]
movdqu xmmword ptr [rbp - 17], xmm0
[... loop etc ...]
pop rbp
ret
__anon_194:
.asciz "asdhfjklsadfhsdjklafhsdajfklsahdfljksadhfkhaksjr"
(godbolt)
Apparently the ++ indeed concatenates the slices on the stack.
Yea that makes sense about the allocator. It complains if I do head: comptime []const u8,
The comptime only applies to the type here and since types are always comptime-known it is indeed redundant. You would have to do comptime head: []const u8, then the compiler would complain that you have to supply an initial comptime-known value for head (which would be immutable at runtime) or pass head as a separate comptime parameter to your functions.
But you probably don’t want head to be comptime here anyways because then you would have to recompile your entire program every time you want to use it.
Thats kinda the idea; I need that hash of the last commit appended to a version string only when I build the application. But now I am not sure if its doable with git binary. But I need it to be able to get hashes when the application does not have .git directory (bsd ports builds, etc).
Ah sorry I’ve missed until now that your code is supposed to be called from a build script. In that case why not use the std.Build API, e.g. std.Build.addSystemCommand()?
head also doesn’t have to be comptime-known within your code, it only needs to be comptime-known from the perspective of the code compiled by the build script, which it will be if it is generated during the build process.
The build script is one option. I also want to cover usage from within the application via same hash(). Tho, maybe I should drop that idea about non-build script use case.