Re: [PATCH 1/5] rust: ptr: add panicking index projection variant

["Alexandre Courbot" <acourbot@nvidia.com>]

On Wed Apr 29, 2026 at 8:29 PM JST, Gary Guo wrote:
> On Wed Apr 29, 2026 at 12:22 PM BST, Alexandre Courbot wrote:
>> On Thu Apr 16, 2026 at 4:57 AM JST, Gary Guo wrote:
>>> There have been a few cases where the programmer knows that the indices are
>>> in bounds but compiler cannot deduce that. This is also
>>> compiler-version-dependent, so using build indexing here can be
>>> problematic. On the other hand, it is also not ideal to use the fallible
>>> variant, as it adds error handling path that is never hit.
>>>
>>> Add a new panicking index projection for this scenario. Like all panicking
>>> operations, this should be used carefully only in cases where the user
>>> knows the index is going to be in bounds, and panicking would indicate
>>> something is catastrophically wrong.
>>>
>>> To signify this, require users to explicitly denote the type of index being
>>> used. The existing two types of index projections also gain the keyworded
>>> version, which will be the recommended way going forward.
>>>
>>> The keyworded syntax also paves the way of perhaps adding more flavors in
>>> the future, e.g. `unsafe` index projection. However, unless the code is
>>> extremely performance sensitive and bounds checking cannot be tolerated,
>>> panicking variant is safer and should be preferred, so it will be left to
>>> future when demand arises.
>>
>> Review nit (no need to respin for this): this patch would probably be
>> easier to review if the renaming of `index` to `build_index` was split
>> into its own patch. It would reduce the diff, while also removing the
>> burden of having to keep in mind that `index` means a different thing
>> depending on whether the line is removed or added. I've found it a bit
>> difficult to keep track of this.
>
> This'll need a respin anyway (to merge with the IO view series) for patch
> logistics reason, so I'll make that change.
>
>>
>>>
>>> Signed-off-by: Gary Guo <gary@garyguo.net>
>>> ---
>>>  rust/kernel/dma.rs            |  3 ++
>>>  rust/kernel/ptr/projection.rs | 98 +++++++++++++++++++++++++++++++++++--------
>>>  2 files changed, 84 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
>>> index 4995ee5dc689..3e4d44749aaf 100644
>>> --- a/rust/kernel/dma.rs
>>> +++ b/rust/kernel/dma.rs
>>> @@ -1207,6 +1207,9 @@ macro_rules! dma_write {
>>>      (@parse [$dma:expr] [$($proj:tt)*] [.$field:tt $($rest:tt)*]) => {
>>>          $crate::dma_write!(@parse [$dma] [$($proj)* .$field] [$($rest)*])
>>>      };
>>> +    (@parse [$dma:expr] [$($proj:tt)*] [[$flavor:ident: $index:expr] $($rest:tt)*]) => {
>>> +        $crate::dma_write!(@parse [$dma] [$($proj)* [$flavor: $index]] [$($rest)*])
>>> +    };
>>>      (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr]? $($rest:tt)*]) => {
>>>          $crate::dma_write!(@parse [$dma] [$($proj)* [$index]?] [$($rest)*])
>>>      };
>>> diff --git a/rust/kernel/ptr/projection.rs b/rust/kernel/ptr/projection.rs
>>> index 140ea8e21617..845811795393 100644
>>> --- a/rust/kernel/ptr/projection.rs
>>> +++ b/rust/kernel/ptr/projection.rs
>>> @@ -26,14 +26,14 @@ fn from(_: OutOfBound) -> Self {
>>>  ///
>>>  /// # Safety
>>>  ///
>>> -/// The implementation of `index` and `get` (if [`Some`] is returned) must ensure that, if provided
>>> -/// input pointer `slice` and returned pointer `output`, then:
>>> +/// The implementation of `index`, `build_index` and `get` (if [`Some`] is returned) must ensure
>>> +/// that, if provided input pointer `slice` and returned pointer `output`, then:
>>>  /// - `output` has the same provenance as `slice`;
>>>  /// - `output.byte_offset_from(slice)` is between 0 to
>>>  ///   `KnownSize::size(slice) - KnownSize::size(output)`.
>>>  ///
>>> -/// This means that if the input pointer is valid, then pointer returned by `get` or `index` is
>>> -/// also valid.
>>> +/// This means that if the input pointer is valid, then pointer returned by `get`, `index` or
>>> +/// `build_index` is also valid.
>>>  #[diagnostic::on_unimplemented(message = "`{Self}` cannot be used to index `{T}`")]
>>>  #[doc(hidden)]
>>>  pub unsafe trait ProjectIndex<T: ?Sized>: Sized {
>>> @@ -42,9 +42,12 @@ pub unsafe trait ProjectIndex<T: ?Sized>: Sized {
>>>      /// Returns an index-projected pointer, if in bounds.
>>>      fn get(self, slice: *mut T) -> Option<*mut Self::Output>;
>>>  
>>> +    /// Returns an index-projected pointer; panic if out of bounds.
>>> +    fn index(self, slice: *mut T) -> *mut Self::Output;
>>
>> This looks like this could have a default implementation:
>>
>>     fn index(self, slice: *mut T) -> *mut Self::Output {
>>         Self::get(self, slice).unwrap()
>>     }
>>
>> I'm sure there is a good reason for now having this though, so at least
>> for my education: why not? :)
>
> It could, but the implementation is going to be dead code.

Would the generated code from this default implementation be worse than
the specialized ones you wrote? I'd have expected the compiler to
optimize things in such a way that they would be equivalent, only with
less code. Haven't looked at it though.