Assignment from struct literal to variable not working

[auwi-nordic]

In the 0.7.0 release it seems like assigning struct literals to variables does not work when not declaring the variable. Or I've misunderstood something about the language (since it seems like a pretty basic use-case)

Here's an example:

const std = @import("std");

pub const PinCnfDir = enum (u1) {
    Input,
    Output
};

pub const PinCnfInput = enum (u1) {
    Connect,
    Disconnect
};

pub const PinCnfPull = enum (u2) {
    Disabled = 0,
    Pulldown = 1,
    Pullup = 3
};

pub const PinCnfDrive = enum (u3) {
    S0S1 = 0,
    H0S1 = 1,
    S0H1 = 2,
    H0H1 = 3,
    D0S1 = 4,
    D0H1 = 5,
    S0D1 = 6,
    H0D1 = 7,
};

pub const PinCnfSense = enum (u2) {
    Disabled = 0,
    High = 2,
    Low = 3
};

pub const PinCnf = packed struct {
    DIR: PinCnfDir = .Input,
    INPUT: PinCnfInput = .Disconnect,
    PULL: PinCnfPull = .Disabled,
    _RESERVED1: u4 = undefined,
    DRIVE: PinCnfDrive = .S0S1,
    _RESERVED3: u5 = undefined,
    SENSE: PinCnfSense = .Disabled,
    _RESERVED4: u14 = undefined
};

pub fn main() !void {
    const stdout = std.io.getStdOut().writer();
    var cnf: PinCnf = undefined;
    var cnf2: PinCnf = PinCnf {.DIR = .Output, .INPUT = .Connect, .PULL = .Disabled, .DRIVE = .S0S1, .SENSE = .Disabled};

    // Case 1: Does not work:
    cnf = PinCnf {.DIR = .Output, .INPUT = .Connect, .PULL = .Disabled, .DRIVE = .S0S1, .SENSE = .Disabled};
    // Case 2: Works:
    cnf = cnf2;

    try stdout.print("Hello \n {} \n", .{cnf});
}

And the result in Case 1 seems similar to not doing any assignment to cnf at all:

PinCnf{ .DIR = PinCnfDir.Input, .INPUT = PinCnfInput.Disconnect, .PULL = PinCnfPull.invalid enum value
???:?:?: 0x2363ad in ??? (???)
/pri/auwi/sw/zig/lib/zig/std/fmt.zig:377:37: 0x235821 in std.fmt.formatType (test)
                try writer.writeAll(@tagName(value));

[Vexu]

This is a bug with packed structs, #3133 is the tracking issue to fix them.

[LemonBoy]

@auwi-nordic The problem here is the use of = undefined initializers in the packed struct, the compiler is mistakenly rounding the field size to a multiple of 8 and generates code that overwrite the trailing fields when it initialized the PinCnf structure using the .{} notation.

The temporary workaround is to initialize the fields to zero rather than undefined, I'll try to send a patch later today to fix the problem.

Keep in mind that the generated code may not do what you expect it to do, the compiler is currently trying to be smart and chops the loads/stores in small ones (mostly u8/u16 here) instead of doing a rmw cycle over the whole 32bit quantity.
The hardware may or may not like this.

[FireFox317]

It is probably also a good idea to always assign 0 to the unused fields, since the hardware may expect that and/or then you clearly define what you write to the register (even if the value doesn't matter).

[auwi-nordic]

@LemonBoy Thanks, initializing to 0 works. Thanks for the heads up regarding loads/stores.. that could be useful to be aware of

@FireFox317 You're absolutely right, defaulting to 0 is better

I also realized that it might be best to do something like this:

pub const PinCnfConfig = struct {
    DIR:   PinCnfDir   = .Input,
    INPUT: PinCnfInput = .Disconnect,
    PULL:  PinCnfPull  = .Disabled,
    DRIVE: PinCnfDrive = .S0S1,
    SENSE: PinCnfSense = .Disabled,
};

pub const PinCnf = packed struct {
    DIR:        PinCnfDir       = 0,
    INPUT:      PinCnfInput     = 0,
    PULL:       PinCnfPull      = 0,
    _RESERVED1: u4              = 0,
    DRIVE:      PinCnfDrive     = 0,
    _RESERVED3: u5              = 0,
    SENSE:      PinCnfSense     = 0,
    _RESERVED4: u14             = 0,

    pub fn set(self: *volatile PinCnf, config: PinCnfConfig) void {
        self.DIR = config.DIR;
        self.INPUT = config.INPUT;
        self.PULL = config.PULL;
        self.DRIVE = config.DRIVE;
        self.SENSE = config.SENSE;
    }
    pub fn reset(self: *volatile PinCnf) void {
        self.set(.{});
    }
};

I'm thinking it might be best to put all hardware register accesses behind abstract functions. That way you can write models of the hardware and run at least parts of your firmware code locally before testing on real hardware