> 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?
— Daniel
