Re: [PATCH v5 04/23] rust: add new `num` module with `PowerOfTwo` type

[John Hubbard]

On 6/12/25 8:07 AM, Boqun Feng wrote:
> On Thu, Jun 12, 2025 at 11:01:32PM +0900, Alexandre Courbot wrote:
...
>> +                #[inline(always)]
>> +                pub const fn align_down(self, value: $t) -> $t {
> 
> I'm late to party, but could we instead implement:
> 
>     pub const fn round_down<i32>(value: i32, shift: i32) -> i32 {
>         value & !((1 << shift) - 1)
>     }
> 
>     pub const fn round_up<i32>(value: i32, shift: i32) -> i32 {
>         let mask = (1 << shift) - 1;
>         value.wrapping_add(mask) & !mask
>     }

Just a naming concern here.

The function name, and the "shift" argument is extremely odd there.
And that's because it is re-inventing the concept of align_down()
and align_up(), but with a misleading name and a hard to understand
"shift" argument.

If you are "rounding" to a power of two, that's normally called
alignment, at least in kernel code. And if you are rounding to the
nearest...integer, for example, that's rounding.

But "rounding" with a "shift" argument? That's a little too 
creative! :) 

> 
> ? It's much harder to pass an invalid alignment with this.

Hopefully we can address argument validation without blowing up
the usual naming conventions.


thanks,
-- 
John Hubbard