Using the struct assignment syntax `myStructValue.* = .{ fields = ... }` is a footgun causing emulator compatibility issues

[pineapplemachine]

One thing I've been working on is another module for interfacing with the GBA's sound registers. I ran into an interesting emulator-dependent bug that turned out to be because of these lines:

gba.sound.ch1_freq.* = .{
    .rate = @intCast(self.rate),
    .reset = reset,
};

Being new to Zig and seeing this similar pattern of assignment to HW registers around the codebase and in examples, I naturally assumed that this would initialize the RHS of the operation as a local with unmentioned fields being default-initialized, and then after that subsequently assign this value to the LHS register. It turns out this isn't what happens. This compiles as a read, modification, and write. And attempting to read from HW registers that are not specifically meant to be read from is definitely a "here be dragons" thing. I haven't had the opportunity to test any of this on real hardware, but even if it might work on hardware, this still breaks common assumptions made by emulators and so remains a problem. (Not to mention that I'm not sure what programmer would read/write this code and expect it to be a read-modify-write instead of only a write.)

I fixed the above by changing it to this:

gba.sound.ch1_freq.* = gba.sound.PulseChannelFrequency {
    .rate = @intCast(self.rate),
    .reset = reset,
};

See relevant discussion in the gbadev Discord here: https://discord.com/channels/768759024270704641/777683711453298698/1378782104212607176

I've verified this issue with 2024.10.0-mach (a dev release of 0.14.0), 0.14.0, 0.14.1, and 0.15.0-dev.643+dc6ffc28b. I'm not sure if this may have behaved differently with earlier versions of Zig?

Having to explicitly name the type is not a very ergonomic feeling API, in my opinion. It also means that, unless there is some other solution, the recent addition of a timers module needs to be refactored to expose the Timer type such that the timer registers can be assigned in this destructured way without causing an unwanted initial read from the register. (Which I am guessing contributed to some of the misbehavior issues discussed in #25.)

I don't know Zig well enough to suggest a specific better alternative just now, unfortunately.

Screenshots of the most relevant Discord discussion:

[vortexofdoom]

Was gonna comment on #25 but I'll just continue the discussion here, since that will probably be able to be completed and closed soon and this looks like it may be a longer term upstream issue. I may have to file an issue in the main zig repo, as that definitely seems like an undesired result of using .{} syntax, if not a regression. I think technically struct literals might be unique types that are duck-typed into coercing, rather than inferring the type, and I can definitely see that interfering with bitpacked structs in particular.

I read the comments in the gbadev discord, definitely some concerning anomalies. I've been a little worried about needing to update the linker script, as it's mostly unchanged from when I adopted the project. The thumb/arm function stuff is extremely annoying, as there's not a simple way to just toggle between them in zig like with attributes in C.

I think as far as the timer module goes, I think this is enough reason to go back to importing the module instead of just the array. Maybe the array can be timers and we have timer be the module like before. That's probably better to make it obvious that timers is an array anyway. I think everything else in there is pub so it should "just work" after that.

The ergonomics of fully specifying the type are certainly unfortunate, but some of it can be mitigated by locally renaming. So instead of your example above it could be something like:

const PulseFreq = gba.sound.PulseChannelFrequency;

gba.sound.ch1_freq.* = PulseFreq{
    .rate = @intCast(self.rate),
    .reset = reset,
};

Not ideal, but better than the alternative. I tend to create constants for anything I'm going to use more than once or twice anyway, so this is only one extra step.

[vortexofdoom]

Looks like this particular issue has already been noticed: https://github.com/ziglang/zig/issues/21249

[pineapplemachine]

The thumb/arm function stuff is extremely annoying, as there's not a simple way to just toggle between them in zig like with attributes in C.

Linking a related issue here: https://github.com/ziglang/zig/issues/22285

It seems like having this would make a lot of things much easier.