make @boolToInt always return a u1 even when the bool is comptime known #7950
Comments
|
Reading this made me wonder why pub fn boolToInt(value: bool) i1 {
return if (value) @as(u1, 1) else @as(u1, 0);
} |
|
The nice thing about |
diff --git a/lib/std/math.zig b/lib/std/math.zig
index 6e7c5c091..f584e2357 100644
--- a/lib/std/math.zig
+++ b/lib/std/math.zig
@@ -1127,7 +1127,7 @@ pub fn maxInt(comptime T: type) comptime_int {
const info = @typeInfo(T);
const bit_count = info.Int.bits;
if (bit_count == 0) return 0;
- return (1 << (bit_count - @boolToInt(info.Int.signedness == .signed))) - 1;
+ return (1 << (bit_count - @as(comptime_int, @boolToInt(info.Int.signedness == .signed)))) - 1;
}
pub fn minInt(comptime T: type) comptime_int {
diff --git a/src/codegen.zig b/src/codegen.zig
index 904fda0de..d4f9b505e 100644
--- a/src/codegen.zig
+++ b/src/codegen.zig
@@ -3590,7 +3590,7 @@ fn Function(comptime arch: std.Target.Cpu.Arch) type {
switch (mcv) {
.immediate => |imm| {
// This immediate is unsigned.
- const U = std.meta.Int(.unsigned, ti.bits - @boolToInt(ti.signedness == .signed));
+ const U = std.meta.Int(.unsigned, ti.bits - @as(comptime_int, @boolToInt(ti.signedness == .signed)));
if (imm >= math.maxInt(U)) {
return MCValue{ .register = try self.copyToTmpRegister(inst.src, mcv) };
}
diff --git a/src/stage1/ir.cpp b/src/stage1/ir.cpp
index e87687302..d8cefcdbf 100644
--- a/src/stage1/ir.cpp
+++ b/src/stage1/ir.cpp
@@ -27845,15 +27845,18 @@ static IrInstGen *ir_analyze_instruction_bool_to_int(IrAnalyze *ira, IrInstSrcBo
return ira->codegen->invalid_inst_gen;
}
+ ZigType *u1_type = get_int_type(ira->codegen, false, 1);
+
if (instr_is_comptime(target)) {
bool is_true;
if (!ir_resolve_bool(ira, target, &is_true))
return ira->codegen->invalid_inst_gen;
- return ir_const_unsigned(ira, &instruction->base.base, is_true ? 1 : 0);
+ IrInstGen *result = ir_const(ira, &instruction->target->base, u1_type);
+ bigint_init_unsigned(&result->value->data.x_bigint, is_true ? 1 : 0);
+ return result;
}
- ZigType *u1_type = get_int_type(ira->codegen, false, 1);
return ir_resolve_cast(ira, &instruction->base.base, target, u1_type, CastOpBoolToInt);
}
|
That's a good point. Check this out though: const std = @import("std");
pub fn main() void {
std.debug.print("{}\n", .{@bitCast(u1, true)}); // 1
std.debug.print("{}\n", .{@bitCast(u1, false)}); // 0
}No branching or control flow needed. I'm not sure if this is UB or not though as we don't have a spec yet, but I'm assuming zig's bool will continue have the same ABI as C's |
|
I would be open to replacing boolToInt with bitCast, but I think I like bitCast a lot more than Andrew does. It would simplify the language a bit though. |
SpexGuy
added
the
proposal
andrewrk
changed the title
|
I think I have not expressed my love of |
|
It seems with #13560 the instruction analyze code gets significantly refactor. @andrewrk could you provide some insights here where is the new code? That is, where we analyze the return type of I found: // src/Air.zig
.bool_to_int => return Type.initTag(.u1),But it seems there is still another path to resolve to |
|
You want |
|
@mlugg That sounds good, lol. Could you link the PR or working branch here? |
|
#15569 - it's an incredibly major refactor, so will be a while before it's merged |
|
Thank you!
Out of curiosity, it seems a solowork by @andrewrk, how do you collaborate there? |
|
The branch is broad enough that it's not too difficult to collaborate without stepping on each others' toes too much - I was eager to get this change in, so I'm doing some bits of work for it on my fork and he's cherry-picking my changes in. |
|
UPDATE: it seems I reverse the boolean logic (oops). Let me try to correct it. original comment@mlugg do you have the concrete commit for implementing this feature? I tried it locally to understand how the Zig compiler works and found that it's more complex than `@boolToInt` is always `u1`.That is, ... which means we need new extra type cast. My change is: diff --git a/src/Sema.zig b/src/Sema.zig
index 1b55f765f..95697b8d8 100644
--- a/src/Sema.zig
+++ b/src/Sema.zig
@@ -18410,8 +18410,8 @@ fn zirBoolToInt(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!A
const operand = try sema.resolveInst(inst_data.operand);
if (try sema.resolveMaybeUndefVal(operand)) |val| {
if (val.isUndef()) return sema.addConstUndef(Type.initTag(.u1));
- const bool_ints = [2]Air.Inst.Ref{ .zero, .one };
- return bool_ints[@boolToInt(val.toBool())];
+ if (val.toBool()) return sema.addConstant(Type.initTag(.u1), Value.initTag(.zero));
+ return sema.addConstant(Type.initTag(.u1), Value.initTag(.one));
}
return block.addUnOp(.bool_to_int, operand);
} |
|
The relevant commit was dropped due to how Andrew merged my changes - I'll just PR this to master shortly. I hadn't tested the relevant change since the branch I originally did it on isn't yet usable. There will probably be a few changes needed to support the new change in code which unintentionally assumed the result type was |
|
Thanks for sharing your works. Now the remaining error is: I may submit a patch for reference if I have time to work it out. Although, this can indicate that such a change is a breaking change that may prevent it from accepted. |
|
The proposal is already accepted. Zig is a v0 language, and as such is very open to breaking changes. |
Can we just guarantee that optimization? It seems weird (right now I came across this). Having But it was Ok having I think having guarantee to optimize away when we can (like guaranteed return value optimization in C++ and something similar in Zig) is better way to deal with issue. |
|
|
If you find that mentioning somewhere in issues (I found that reply from Andrew). I bet 95% will write (including myself) const val = 4;
if(val == 0) false else true |
A core design pillar of the language is that code that works at runtime should be able to work at compile time (barring significant metaprogramming or weird pointer shenanigans). This rule is broken here:
This code will compile with a runtime known
valuebut not with a comptime knownvalue. The workaround is to use@bitCast(i1, @as(u1, @boolToInt(value))), but this feels like a "trick" to make the code safe for use at comptime. The fewer of those we have, the better.Instead,
@boolToIntshould return a comptime known u1 when the parameter is comptime known, to be consistent.The text was updated successfully, but these errors were encountered: