On Thu Sep 18, 2025 at 8:13 PM CEST, Joel Fernandes wrote: > On Thu, Sep 18, 2025 at 03:02:11PM +0000, Alice Ryhl wrote: >> Using build_assert! to assert that offsets are in bounds is really >> fragile and likely to result in spurious and hard-to-debug build >> failures. Therefore, build_assert! should be avoided for this case. >> Thus, update the code to perform the check in const evaluation instead. > > I really don't think this patch is a good idea (and nobody I spoke to thinks > so). Not only does it mess up the user's caller syntax completely, it is also
I appreacite you raising the concern, but I rather have other people speak up themselves. > super confusing to pass both a generic and a function argument separately. Why? We assert that the offset has to be const, whereas the value does not have this requirement, so this makes perfect sense to me. (I agree that it can look unfamiliar at the first glance though.) > Sorry if I knew this would be the syntax, I would have objected even when we > spoke :) > > I think the best fix (from any I've seen so far), is to move the bindings > calls of offending code into a closure and call the closure directly, as I > posted in the other thread. I also passed the closure idea by Gary and he > confirmed the compiler should behave correctly (I will check the code gen > with/without later). Gary also provided a brilliant suggestion that we can > call the closure directly instead of assigning it to a variable first. That > fix is also smaller, and does not screw up the users. APIs should fix issues > within them instead of relying on user to work around them. This is not a workaround, this is an idiomatic solution (which I probably should have been doing already when I introduced the I/O code). We do exactly the same thing for DmaMask::new() [1] and we agreed on doing the same thing for Alignment as well [2]. [1] https://rust.docs.kernel.org/kernel/dma/struct.DmaMask.html#method.new [2] https://lore.kernel.org/rust-for-linux/20250908-num-v5-1-c0f2f681e...@nvidia.com/
