Re: [PATCH v5 21/38] rust: ptr: add const_align_up() and enable inline_const feature

[Gary Guo]

On Thu Mar 5, 2026 at 7:07 AM GMT, Alexandre Courbot wrote:
> On Thu Mar 5, 2026 at 10:31 AM JST, John Hubbard wrote:
>> On 3/4/26 5:23 PM, Alexandre Courbot wrote:
>>> On Thu Mar 5, 2026 at 4:14 AM JST, John Hubbard wrote:
>>>> On 3/4/26 11:04 AM, Gary Guo wrote:
>>>>> On Wed Mar 4, 2026 at 6:53 PM GMT, John Hubbard wrote:
>>>>>> On 3/4/26 3:18 AM, Gary Guo wrote:
>>>>>>> On Wed Mar 4, 2026 at 3:47 AM GMT, John Hubbard wrote:
>>>>>> ...
>>>>>> +#[inline(always)]
>>>>>> +pub const fn const_align_up<const ALIGN: usize>(value: usize) -> Option<usize> {
>>>>>> +    const { assert!(ALIGN.is_power_of_two(), "ALIGN must be a power of two") };
>>>>>> +    match value.checked_add(ALIGN - 1) {
>>>>>> +        Some(v) => Some(v & !(ALIGN - 1)),
>>>>>> +        None => None,
>>>>>> +    }
>>>>>> +}
>>>>>
>>>>> I think your signature should probably just be
>>>>>
>>>>> pub const fn const_align_up(value: usize, align: Alignment) -> Option<usize> {
>>>>>     ...
>>>>> }
>>>>>
>>>>
>>>> OK yes that's a bit nicer. I've done that for v6, thanks!
>>> 
>>> Hold on a bit - if we are purposing this new method for use in const
>>> contexts, what use do we have for a `None` return value? By definition
>>> we would know both `value` and `align` and thus the result is
>>> deterministic.
>>> 
>>> We do have an alignment method for non-const contexts already. Gary's
>>> initial comment was:
>>> 
>>>> Either this function is always used in const context, in which case
>>>> you take `ALIGN` as normal function parameter and use `build_assert` and
>>>> `build_error`
>>> 
>>> So why not make both arguments generic in this new method, and fail at
>>> build in case of overflow? 
>>
>> At this point, it is completely impossible to write a patch that complies
>> with Gary, Danilo, and Alex. It's all over the map.
>
> IIUC it is possible. Let's summarize the constraints:
>
> - Gary wants to avoid a panic in case this gets called at runtime,
> - Danilo suggested returning a Result that can be discarded in const
>   context (but took that suggestion back as we already have methods for
>   non-const contexts and thus wouldn't bring any benefit),
> - I also pointed out that there is not reason to have a failure path for
>   const context and suggested two generic arguments.
>
> So here is what I had in mind, if using a standalone function:
>
>   pub const fn const_align_up<const ALIGN: usize, const VALUE: usize>() -> usize {
>       const { assert!(ALIGN.is_power_of_two(), "ALIGN must be a power of two") };
>       const {
>           assert!(
>               VALUE <= usize::MAX - (ALIGN - 1),
>               "requested alignment would overflow"
>           )
>       };
>
>       (VALUE + (ALIGN - 1)) & !(ALIGN - 1)
>   }

Eh, no, please don't start put everything into const generics params. These are
severely limited in usability, you can only refer to actual constant values, and
won't be able to use them in, say, const functions.

If we're going down this route I'd just want

    pub const fn const_align_up(align: usize, value: usize) -> usize

and use build asserts inside. If this is only used in const, then using
`build_assert!` is perfectly fine.

I think John just want to express `usize.align_up(Alignment)` in const context,
which we can't do with stable features only right now, hence I sugggested a
specific const function that has the same signature as the extension trait
method.

Returning a `Option` isn't an issue in const contexts, you can just use
`Option::unwrap` which is const (might need to enable a feature in 1.78, but it
is stable for a while now).

So you just have

    const TEST_ALIGN: usize = const_align_up(10, Alignment::new::<256>()).unwrap();

which would become

    const TEST_ALIGN: usize = 10.align_up(Alignmnet::new::<256>()).unwrap();

when we have const trait impl.

>
>   const TEST_ALIGN: usize = const_align_up::<256, 10>();
>
> This uses purely const asserts, but you have to work with two `usize`
> arguments. The version below looks a bit nicer as it leverages the
> power-of-two invariant of `Alignment`:
>
>   impl Alignment {
>       const fn const_align_up(self, value: usize) -> usize {
>           build_assert!(value <= usize::MAX - !self.mask());
>
>           (value + !self.mask()) & self.mask()
>       }

This is fine, too, although I think just returning an `Option` and ask user to
unwrap it in const eval is better.

Best,
Gary

>   }
>
>   const TEST_ALIGN2: usize = Alignment::new::<256>().const_align_up(10);
>
> It has to trade the const asserts for `build_assert`, which could cause
> these cryptic error messages if called in a non-const context, so we
> should document that this is only to be called in const contexts. But
> otherwise it fits the bill and looks reasonable imho.
>
> Unfortunate that we cannot make it generic against all integer types
> without `const_trait_impl`, but generating `const_align_usize_up`,
> `const_align_u32_up`... etc using a macro should be doable if needed.
>
> Oh and if this cannot reach consensus I am ok with just dropping this
> patch for now and doing something like this in the next one:
>
>   const SZ_128K_ALIGN_MASK: usize = Alignment::new::<SZ_128K>().mask();
>
>   const PMU_RESERVED_SIZE: usize = SZ_8M + SZ_16M + SZ_4K;
>   // Align to 128K.
>   const PMU_RESERVED_SIZE_ALIGNED: u32 = num::usize_into_u32::<
>       { (PMU_RESERVED_SIZE + !SZ_128K_ALIGN_MASK) & SZ_128K_ALIGN_MASK },
>   >();