On Sat, Jul 12, 2025 at 10:49 PM Daniel Almeida
<daniel.almeida@xxxxxxxxxxxxx> wrote:
>
>
>
> > On 12 Jul 2025, at 17:03, Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
> >
> > On Sat, Jul 12, 2025 at 6:27 PM Daniel Almeida
> > <daniel.almeida@xxxxxxxxxxxxx> wrote:
> >>
> >> Hi Alice,
> >>
> >>> On 4 Jul 2025, at 04:42, Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
> >>>
> >>> On Fri, Jul 04, 2025 at 08:14:11AM +0200, Daniel Sedlak wrote:
> >>>> Hi Daniel,
> >>>>
> >>>> On 7/3/25 9:30 PM, Daniel Almeida wrote:
> >>>>> +/// Flags to be used when registering IRQ handlers.
> >>>>> +///
> >>>>> +/// They can be combined with the operators `|`, `&`, and `!`.
> >>>>> +#[derive(Clone, Copy, PartialEq, Eq)]
> >>>>> +pub struct Flags(u64);
> >>>>
> >>>> Why not Flags(u32)? You may get rid of all unnecessary casts later, plus
> >>>> save some extra bytes.
> >>>
> >>> It looks like the C methods take an `unsigned long`. In that case, I'd
> >>> probably write the code to match that.
> >>>
> >>> pub struct Flags(c_ulong);
> >>>
> >>> and git rid of the cast when calling bindings::request_irq.
> >>>
> >>> As for all the constants in this file, maybe it would be nice with a
> >>> private constructor that uses the same type as bindings to avoid the
> >>> casts?
> >>>
> >>> impl Flags {
> >>> const fn new(value: u32) -> Flags {
> >>> ...
> >>> }
> >>> }
> >>
> >>
> >> Sure, but what goes here? This has to be "value as c_ulong” anyways so it
> >> doesn’t really reduce the number of casts.
> >>
> >> We should probably switch to Flags(u32) as Daniel Sedlak suggested. Then
> >> it’s a matter of casting once for bindings::request_irq().
> >
> > IMO the advantage of doing it here is that we can fail compilation if
> > the cast is out of bounds, whereas the other cast is at runtime so we
> > can't do that.
> >
> > Alice
>
> I’m not sure I am following. How is this compile-time checked?
>
> >>> impl Flags {
> >>> const fn new(value: u32) -> Flags {
> >>> Self(value as c_ulong)
> >>> }
>
> Or perhaps I misunderstood you?
Well, that particular implementation would not be. But you could
implement it to compile-time check.
Alice
