mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-04-19 22:10:29 +00:00
build: refactor checkGhosttyHEnum to use @hasDecl for Windows compatibility (#11813)
### This is it! This one (and the other stacked PRs) and #11782 should finally give a clean test run on Windows! ## Summary - Increase `@setEvalBranchQuota` from 1M to 10M (too much? how much is too much?) in `checkGhosttyHEnum` (src/lib/enum.zig) - Fixes the only remaining test failure on Windows MSVC: `ghostty.h MouseShape` ## Context This one was fun! Claude started blabbering, diminishing returns it said. It couldn't figure out. So I called Dario and it worked. Nah, much easier than that. On MSVC, the translate-c output for `ghostty.h` is ~360KB with ~2173 declarations (vs ~112KB / ~1502 on Linux/Mac) because `<sys/types.h>` and `<BaseTsd.h>` pull in Windows SDK headers. The `checkGhosttyHEnum` function uses a nested `inline for` (enum fields x declarations) with comptime string comparisons. For MouseShape (34 variants), this generates roughly 34 x 2173 x ~20 = ~1.5M comptime branches, exceeding the 1M quota. The failure was confusing because it presented as a runtime error ("ghostty.h is missing value for GHOSTTY_MOUSE_SHAPE_DEFAULT") rather than a compile error. The constants exist in the translate-c output and the test compiles, but the comptime loop silently stops matching when it hits the branch limit, so `set.remove` is never called and the set reports all entries as missing at runtime. ## How we found it The translate-c output clearly had all 34 GHOSTTY_MOUSE_SHAPE_* constants, yet the test reported all of them as missing. I asked Claude to list 5 hypotheses (decl truncation, branch quota, string comparison bug, declaration ordering, field access failure) and to write 7 targeted POC tests in enum.zig to isolate each step of `checkGhosttyHEnum`: 1. POC1-2: Module access and declaration count (both passed) 2. POC3: `@hasDecl` for the constant (passed) 3. POC4: Direct field value access (passed) 4. POC5: `inline for` over decls with string comparison - **compile error: "evaluation exceeded 1000 backwards branches"** 5. POC6: Same but with 10M quota (passed) 6. POC7: Full `checkGhosttyHEnum` clone with 10M quota - **passed, confirming the fix** POC5 was the key: the default 1000 branch limit for test code confirmed the comptime budget mechanism. The existing 1M quota in `checkGhosttyHEnum` was enough for Linux/Mac (1502 declarations) but not for MSVC (2173 declarations) with larger enums. ## Stack Stacked on 016-windows/fix-libcxx-msvc. ## Test plan ### Cross-platform results (`zig build test` / `zig build -Dapp-runtime=none test` on Windows) | | Windows | Linux | Mac | |---|---|---|---| | **BEFORE** (016,ce9930051) | FAIL - 49/51, 2630/2654, 1 test failed, 23 skipped | PASS - 86/86, 2655/2678, 23 skipped | PASS - 160/160, 2655/2662, 7 skipped | | **AFTER** (017,68378a0bb) | FAIL - 49/51, 2631/2654, 23 skipped | PASS - 86/86, 2655/2678, 23 skipped | PASS - 160/160, 2655/2662, 7 skipped | ### Windows: what changed (2630 -> 2631 tests, MouseShape fixed) **Fixed by this PR:** - `ghostty.h MouseShape` test - was failing because comptime branch quota exhaustion silently prevented the inline for loop from matching any constants **Remaining failure (pre-existing, unrelated):** - `config.Config.test.clone can then change conditional state` - segfaults (exit code 3) on Windows. We investigated this and it looked familiar.. cherry-picking the `CommaSplitter `fix from PR #11782 resolved it! The backslash path handling in `CommaSplitter `breaks theme path parsing on Windows, which is exactly what that PR addresses. So once that lands, we should be in a good place... ready to ship to Windows users! (just kidding) ### Linux/macOS: no regressions Identical pass counts and test results before and after. ## What I Learnt - Comptime branch quota exhaustion in Zig does not always surface as a clean compile error. When it happens inside an `inline for` loop with `comptime` string comparisons that gate runtime code (like `set.remove`), the effect is that matching code is silently not generated. The test compiles and runs, but the runtime behavior is wrong because the matching branches were never emitted. This makes the failure look like a data issue (missing declarations) rather than a compile budget issue. - When debugging comptime issues, writing small isolated POC tests that exercise each step of the failing function independently is very effective. It took 7 targeted tests to pinpoint the exact failure point. - Cross-platform translate-c outputs can vary significantly in size. On MSVC, system headers are much larger than on Linux/Mac, which affects comptime budgets for any code that iterates over translated module declarations.
This commit is contained in:
@@ -105,30 +105,27 @@ pub fn checkGhosttyHEnum(
|
||||
try std.testing.expect(info.@"enum".tag_type == c_int);
|
||||
try std.testing.expect(info.@"enum".is_exhaustive == true);
|
||||
|
||||
@setEvalBranchQuota(1000000);
|
||||
@setEvalBranchQuota(100_000);
|
||||
|
||||
const c = @import("ghostty.h");
|
||||
|
||||
var set: std.EnumSet(T) = .initFull();
|
||||
|
||||
const c_decls = @typeInfo(c).@"struct".decls;
|
||||
const enum_fields = info.@"enum".fields;
|
||||
|
||||
inline for (enum_fields) |field| {
|
||||
const upper_name = comptime u: {
|
||||
var buf: [128]u8 = undefined;
|
||||
break :u std.ascii.upperString(&buf, field.name);
|
||||
const expected_name: *const [prefix.len + field.name.len]u8 = comptime e: {
|
||||
var buf: [prefix.len + field.name.len]u8 = undefined;
|
||||
@memcpy(buf[0..prefix.len], prefix);
|
||||
for (buf[prefix.len..], field.name) |*d, s| {
|
||||
d.* = std.ascii.toUpper(s);
|
||||
}
|
||||
break :e &buf;
|
||||
};
|
||||
|
||||
inline for (c_decls) |decl| {
|
||||
if (!comptime std.mem.startsWith(u8, decl.name, prefix)) continue;
|
||||
|
||||
const suffix = decl.name[prefix.len..];
|
||||
|
||||
if (!comptime std.mem.eql(u8, suffix, upper_name)) continue;
|
||||
|
||||
std.testing.expectEqual(field.value, @field(c, decl.name)) catch |e| {
|
||||
std.log.err(@typeName(T) ++ " key " ++ field.name ++ " does not have the same backing int as " ++ decl.name, .{});
|
||||
if (@hasDecl(c, expected_name)) {
|
||||
std.testing.expectEqual(field.value, @field(c, expected_name)) catch |e| {
|
||||
std.log.err(@typeName(T) ++ " key " ++ field.name ++ " does not have the same backing int as " ++ expected_name, .{});
|
||||
return e;
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user