Adding / counting bools

[kj4tmp]

Is there a better way to express the bool adding / counting math I need to do here?

pub const DLStatus = packed struct {
    dls_user_operational: bool,
    dls_user_watchdog_ok: bool,
    extended_link_detection: bool,
    _reserved: u1 = 0,

    port0_physical_link: bool,
    port1_physical_link: bool,
    port2_physical_link: bool,
    port3_physical_link: bool,

    port0_loop_internal: bool,
    port0_rx_signal_detected: bool,
    port1_loop_internal: bool,
    port1_rx_signal_detected: bool,
    port2_loop_internal: bool,
    port2_rx_signal_detected: bool,
    port3_loop_internal: bool,
    port3_rx_signal_detected: bool,

    pub fn topology(self: DLStatus) Topology {
        const port0_active = self.port0_rx_signal_detected and !self.port0_loop_internal;
        const port1_active = self.port1_rx_signal_detected and !self.port1_loop_internal;
        const port2_active = self.port2_rx_signal_detected and !self.port2_loop_internal;
        const port3_active = self.port3_rx_signal_detected and !self.port3_loop_internal;

        const n_active_ports: u3 =
            @as(u3, @intFromBool(port0_active)) +
            @as(u3, @intFromBool(port1_active)) +
            @as(u3, @intFromBool(port2_active)) +
            @as(u3, @intFromBool(port3_active));

        return switch (n_active_ports) {
            0 => return .end,
            1 => return .end,
            2 => return .line,
            3 => return .wye,
            4 => return .cross,
            else => unreachable,
        };
    }
};

pub const Topology = enum {
    end,
    line,
    wye,
    cross,
};

I would prefer comptime unreachable over runtime unreachable.

[dimdin]

const ports = [_]bool{ port0_active, port1_active, port2_active, port3_active };
var n_active_ports: u3 = 0;
inline for (ports) |b| {
    n_active_ports += @intFromBool(b);
}

[tholmes]

test "countTrues" {
	const vec: @Vector(4, bool) = .{
		true,
		true,
		false,
		true,
	};
	try std.testing.expectEqual(@as(usize, 3), std.simd.countTrues(vec));
}

The returned value isn’t actually usize but std.math.IntFittingRange() with the vector’s length. (So, u3).
This isn’t actually relevant in our case, though, since we still can’t comptime-assert that the value can’t be greater than 4… sadly.

[ericlang]

Interesting problem. This one - if you stuff the ports in a substruct - does at least count easier.
But still the u3 problem.

const Ports = packed struct(u4) {
    port0_physical_link: bool,
    port1_physical_link: bool,
    port2_physical_link: bool,
    port3_physical_link: bool,

    fn from_int(u: u4) Ports {
        return @bitCast(u);
    }

    fn open(self: Ports) u3 {
        const u: u4 = @bitCast(self);
        return @popCount(u);
    }
};

[pancelor]

allow integer types to be any range · Issue #3806 · ziglang/zig · GitHub would make this easy, particularly with @popCount. But that doesn’t really help, since #3806 isn’t implemented (or accepted? unclear)

[floooh]

I can’t offer a good solution, but problems like this is basically why I sticked with bit twiddling on integers for chip IO pins in my emulator project instead of packed-structs of bools. E.g. with bits in an integer you have all the bitwise operations (including left- and right-shift) at your disposal which usually makes the code both more compact and readable (if you’re “fluent” in bit twiddling ops at least).

[Plebosaur]

Popcount would work great here, I am not sure if you have to define the backing integer, I will do it here for demonstration purposes:

example:

const S = packed struct (u8) {
    b0: bool = false,
    b1: bool = false,
    b2: bool = false,
    b3: bool = false,
    b4: bool = false,
    b5: bool = false,
    b6: bool = false,
    b7: bool = false,

    pub fn count(s: S) u8 {
        const backing: u8 = @bitCast(s);
        return @popCount(backing);
    }
    // You can do this for filters,
    // let's say I only want b1, b2, and b4:
    pub fn count2(s: S) u8 {
        const mask: u8 = comptime @bitCast(S{
            .b1 = true,
            .b2 = true,
            .b4 = true,
        });
        const backing: u8 = @bitCast(s);
        return @popCount(backing & mask);
    }
};

test {
    const s: S = .{
        .b0 = true,
        .b2 = true,
        .b4 = true,
        .b6 = true,
    };
    try std.testing.expect(s.count() == 4);
    try std.testing.expect(s.count2() == 2);
}

EDIT: It’s not clear how you can use this for your use case with my first example, so here’s a better example, it should be trivial to solve with this I believe:

    // Let's say that I want the following:
    // increment if b0 and !b1
    // increment if b2 and !b3
    pub fn count3(s: S) u8 {
        const backing: u8 = @bitCast(s);
        const ok02: u8 = blk: {
            const mask: u8 = @bitCast(S{
                .b0 = true,
                .b2 = true,
            });
            break :blk mask & backing;
        };
        const notOk13: u8 = blk: {
            const mask: u8 = @bitCast(S{
                .b1 = true,
                .b3 = true,
            });
            break :blk ~(mask & backing);
        };
        return @popCount(ok02 & (notOk13 >> 1));
    }




test {
    const s1: S = .{
        .b0 = true,
        .b2 = true,
    };
    try std.testing.expect(s1.count3() == 2);
    const s2: S = .{
        .b0 = true,
        .b1 = true,
        .b2 = true,
        .b3 = true,
    };
    try std.testing.expect(s2.count3() == 0);
    const s3: S = .{
        .b0 = true,
        .b2 = true,
        .b3 = true,
    };
    try std.testing.expect(s3.count3() == 1);

}

[Plebosaur]

This ditches the unreachable too:

const S = packed struct (u8) {
    port0_loop_internal: bool = false,
    port0_rx_signal_detected: bool = false,
    port1_loop_internal: bool = false,
    port1_rx_signal_detected: bool = false,
    port2_loop_internal: bool = false,
    port2_rx_signal_detected: bool = false,
    port3_loop_internal: bool = false,
    port3_rx_signal_detected: bool = false,
    
    const okBits: u8 = @bitCast(S{
        .port0_rx_signal_detected = true,
        .port1_rx_signal_detected = true,
        .port2_rx_signal_detected = true,
        .port3_rx_signal_detected = true,
    });
    const notOkBits: u8 = @bitCast(S{
        .port0_loop_internal = true,
        .port1_loop_internal = true,
        .port2_loop_internal = true,
        .port3_loop_internal = true,
    });
    
    const Topology = enum(u2) {
        end,
        line,
        wye,
        cross,
    };

    fn bitStuff(s: S) Topology {
        const bits: u8 = @bitCast(s);
        const ok: u8 = (bits & okBits) >> 1;
        const notOk: u8 = ~(bits & notOkBits);
        
        const final = ok & notOk;
        // example: 01010101 with max 4 ones.
        
        // This trims the first 1.
        // so we would get 01010100 max value
        const trim1 = final & (final -% 1);
        const count: u2 = @intCast(@popCount(trim1));
        // from 0 to 3
        // If you don't like the intcast, which is unreachable in disguise
        // you would have to put these 1 bits into a u3 and then popcount
        
        // Other way to do this:
        // count: u2 = @intCast(@popCount(final) -| 1);
        
        return @enumFromInt(count);
    }
};

Edit: Forgot to bitcast, nonetheless you can take it from here.

[alanza]

i don’t really mean this as anything like a call-out, so i’m sorry if this comes on a little strong, but: why write this reply? many topics in Help ask about an “ergonomic” or “Zig” way to do things and there’s a notable minority of replies to such topics that basically communicate “just give up” and they always rub me slightly the wrong way…

[mg979]

@kj4tmp

        const n_active_ports: u3 =
            @as(u3, @intFromBool(port0_active)) +
            @as(u3, @intFromBool(port1_active)) +
            @as(u3, @intFromBool(port2_active)) +
            @as(u3, @intFromBool(port3_active));

I’d do

        const n_active_ports: u3 =
            i(port0_active) +
            i(port1_active) +
            i(port2_active) +
            i(port3_active);

...

    fn i(port: bool) u3 {
        return @intFromBool(port);
    }

If you keep the helper in scope it doesn’t bother anybody else.

Edit: replied to the wrong post.

[mitteneer]

Idk if Vectors are usable for you, but it’s a fun idea I have, so I’m rolling w/ it (and I don’t think it should be too hard to adapt to what you need anyways)

const detected: @Vector(4,bool) = .{
    self.port0_rx_signal_detected,
    self.port1_rx_signal_detected,
    self.port2_rx_signal_detected,
    self.port3_rx_signal_detected,
};

const loop: @Vector(4,bool) = .{
    !self.port0_loop_internal,
    !self.port1_loop_internal,
    !self.port2_loop_internal,
    !self.port3_loop_internal,
};

const active: u2 = @max(0,@popCount(detected ^ loop) -| 1);
return switch (active) {
    0 => .end,
    1 => .line,
    2 => .wye,
    3 => .cross,
};

Edit: Seems like I’m mixing logical operations and bit/math operations a little more than I should be (can’t use and on @Vector(n, bool)). Vectors are a little finicky like that. You would need some @intFromBools thrown around in there, but the idea remains the same

Edit 2: I also misunderstood how @popCount works, even beyond the necessary correct from Plebosaur. I’m sure there’s something functional in here, but I’m not quite there unfortunately

Edit 3: For travelers from the future: It returns a Vector corresponding to each entry. The least dumb way I can see to make this work as it’s written is to replace the definition of active with:

const active: u2 = @max(0,std.simd.countTrues(detected ^ loop) -| 1)

You can also @bitCast the xor to a u4 and then use @popCap, but if you’re okay with std, I’d prob just use countTrues. All said, consider me thoroughly (and happily) nerd-sniped

[Plebosaur]

use -|1 instead of @max(0, val -1) because popcount returns unsigned value, subtraction by 1 will give illegal behaviour if val == 0.

[Tekla]

I’d personally write it as:

pub const Topology = enum { end, line, wye, cross };

const Port = packed struct(u3) {
    link: bool = false,
    loop: bool = false,
    signal: bool = false,

    pub fn isActive(self: Port) bool {
        return self.signal and !self.loop;
    }

    pub fn isActiveAsNumber(self: Port) u1 {
        return if (isActive(self)) 1 else 0;
    }
};

pub const DLStatus = packed struct(u16) {
    dls_user_operational: bool = false,
    dls_user_watchdog_ok: bool = false,
    extended_link_detection: bool = false,
    _reserved: u1 = 0,

    port0: Port = .{},
    port1: Port = .{},
    port2: Port = .{},
    port3: Port = .{},

    inline fn activePortsCount(self: DLStatus) u8 {
        var sum: u8 = 0;
        sum += self.port0.isActiveAsNumber();
        sum += self.port1.isActiveAsNumber();
        sum += self.port2.isActiveAsNumber();
        sum += self.port3.isActiveAsNumber();
        return sum;
    }

    pub fn topology(self: DLStatus) Topology {
        return switch (activePortsCount(self)) {
            0, 1 => return .end,
            2 => return .line,
            3 => return .wye,
            4 => return .cross,
            else => unreachable,
        };
    }
};

I’d love to have an array of ports here but it’s not possible. Maybe I’ll try vectors later on.
Same size as the original btw, which seems important given your code.

[kj4tmp]

@popCount will not operate on a vector of bools, only vectors of integers. And it returns one integer for each element in the given vector.

const active_ports: @Vector(4, bool) = .{
    port0_active,
    port1_active,
    port2_active,
    port3_active,
};
const n_active_ports: u3 = @popCount(active_ports);

src/module/esc.zig:299:46: error: expected vector of integers; found vector of 'bool'
        const n_active_ports: u3 = @popCount(active_ports);
                                             ^~~~~~~~~~~~

so if I use @popCount it needs to be in a packed struct:

const active_ports: packed struct(u4) {
    port0: bool,
    port1: bool,
    port2: bool,
    port3: bool,
} = .{
    .port0 = port0_active,
    .port1 = port1_active,
    .port2 = port2_active,
    .port3 = port3_active,
};
const n_active_ports: u3 = @popCount(@as(u4, @bitCast(active_ports)));

[kj4tmp]

Thanks everyone for the ideas. I think @popCount communicates intent precisely here so I’m going to use it.

1. I’m primarily concerned with the code being correct, even in the face of future language changes, and changes to the std lib.
2. Hence reducing std lib usage.
3. Hence code being procedural, to align with language / style of specifications I am working against.

The remaining risks I’ve identified:

1. I could have chosen the wrong integer for the count (u3) (its not but I would prefer the code to reflect all decisions / information I’ve made in my head).

In response to this are two options:

I could include comptime assertions to check my integer being used:

const PortCount = u3;
comptime assert(std.math.maxInt(PortCount) >= 4);
comptime assert(std.math.minInt(PortCount) <= 0);

const n_active_ports: PortCount =
    @as(PortCount, @intFromBool(port0_active)) +
    @as(PortCount, @intFromBool(port1_active)) +
    @as(PortCount, @intFromBool(port2_active)) +
    @as(PortCount, @intFromBool(port3_active));

I could use @popCount, which will give me an integer fitting the range:

const active_ports: packed struct(u4) {
    port0: bool,
    port1: bool,
    port2: bool,
    port3: bool,
} = .{
    .port0 = port0_active,
    .port1 = port1_active,
    .port2 = port2_active,
    .port3 = port3_active,
};
const n_active_ports: u3 = @popCount(@as(u4, @bitCast(active_ports)));

I like the second implementation because its just less math operators, and fewer math operators means fewer places I could overflow and panic if I got it wrong.

Something I don’t like about this implementation is it is not resistant to typos in constructing the struct. (I could inadvertently use the same active status twice). Like this:

const active_ports: packed struct(u4) {
            port0: bool,
            port1: bool,
            port2: bool,
            port3: bool,
        } = .{
            .port0 = port0_active,
            .port1 = port0_active,
            .port2 = port2_active,
            .port3 = port3_active,
        };
        const n_active_ports: u3 = @popCount(@as(u4, @bitCast(active_ports)));

We can further reduce the lines using a bool vector:

const active_ports: @Vector(4, bool) = .{
    port0_active,
    port1_active,
    port2_active,
    port3_active,
};
const n_active_ports: u3 = @popCount(@as(u4, @bitCast(active_ports)));

Another change is to not use else in the switch:

return switch (n_active_ports) {
    0 => return .end,
    1 => return .end,
    2 => return .line,
    3 => return .wye,
    4 => return .cross,
    5, 6, 7 => unreachable, // there are only 4 ports.
};

This makes me resistant to future language changes to @popCount if they ever occur, giving me a compile error to re-review the semantics in a future zig version.

Here is the final implementation:

pub fn topology(self: DLStatus) Topology {
    const port0_active = self.port0_rx_signal_detected and !self.port0_loop_internal;
    const port1_active = self.port1_rx_signal_detected and !self.port1_loop_internal;
    const port2_active = self.port2_rx_signal_detected and !self.port2_loop_internal;
    const port3_active = self.port3_rx_signal_detected and !self.port3_loop_internal;

    const active_ports: @Vector(4, bool) = .{
        port0_active,
        port1_active,
        port2_active,
        port3_active,
    };
    const n_active_ports: u3 = @popCount(@as(u4, @bitCast(active_ports)));

    return switch (n_active_ports) {
        0 => return .end,
        1 => return .end,
        2 => return .line,
        3 => return .wye,
        4 => return .cross,
        5, 6, 7 => unreachable, // there are only 4 ports.
    };
}

[floooh]

that basically communicate “just give up”

It’s not meant as a ‘just give up’ post, but as a suggestion that maybe traditional bit twiddling might be a better option than being too fixated on booleans (since bits in an integer are more flexible than booleans when they need to be manipulated in groups, because suddenly you have all the language’s bitwise operations at your disposal).

Zig’s packed structs are very close to being helpful if there would be language sugar which allows the booleans in a packed struct to be manipulated with bitwise operations (and I actually tried that first in my emulators before going back to regular integers).

[Plebosaur]

There’s a pattern with packed unions that I found gives the best of both worlds:

const S = packed union(u3) {
    breakdown: packed struct(u3) {
        b0: bool,
        b1: bool,
        b2: bool,
    },
    seed: u3,
};

I found the reduction in friction from using .seed instead of @bitCast() (often have to use @as) makes this very enjoyable to use.

[AndrewKraevskii]

This will definitly help with reduction of friction https://codeberg.org/ziglang/zig/issues/35602

[alanza]

@floooh

if there would be language sugar which allows the booleans in a packed struct to be manipulated with bitwise operations

I could see something like this being a pretty easy snippet to write

const Thing = packed struct(u8) {
    fleeble: bool,
    florp: bool,
    fizz: bool,
    buzz: bool,
    foo: bool,
    bar: bool,
    bat: bool,
    baz: bool,
    
    pub fn merge(this: *Thing, with: Thing) void {
        // depends on accepted proposal
        this.* = @fromBackingInt(@backingInt(this.*) | @backingInt(with));

        // works today
        const curr: u8 = @bitCast(this.*);
        const other: u8 = @bitCast(with);
        this.* = @bitCast(curr | other);
    }
};