| Message ID | 20250211-module-params-v3-v6-2-24b297ddc43d@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rust: extend `module!` macro with integer parameter support | expand |
| Context | Check | Description |
|---|---|---|
| mcgrof/vmtest-modules-next-PR | fail | PR summary |
| mcgrof/vmtest-modules-next-VM_Test-3 | fail | Logs for cleanup / Archive results and cleanup |
| mcgrof/vmtest-modules-next-VM_Test-4 | fail | Logs for setup / Setup kdevops environment |
| mcgrof/vmtest-modules-next-VM_Test-0 | success | Logs for Run CI tests |
| mcgrof/vmtest-modules-next-VM_Test-5 | fail | Logs for setup / Setup kdevops environment |
| mcgrof/vmtest-modules-next-VM_Test-2 | fail | Logs for cleanup / Archive results and cleanup |
| mcgrof/vmtest-modules-next-VM_Test-1 | success | Logs for Run CI tests |
On Tue, 11 Feb 2025 16:57:36 +0100 Andreas Hindborg <a.hindborg@kernel.org> wrote: > The `Index` implementation on `BStr` was lost when we switched `BStr` from > a type alias of `[u8]` to a newtype. This patch adds back `Index` by > implementing `Index` for `BStr` when `Index` would be implemented for > `[u8]`. > > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org> > --- > rust/kernel/str.rs | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs > index 002dcddf7c768..1eb945bed77d6 100644 > --- a/rust/kernel/str.rs > +++ b/rust/kernel/str.rs > @@ -114,6 +114,17 @@ fn eq(&self, other: &Self) -> bool { > } > } > > +impl<Idx> Index<Idx> for BStr > +where > + Idx: core::slice::SliceIndex<[u8], Output = [u8]>, I think I'd prefer [T]: Index<Idx>, here. > +{ > + type Output = Self; > + > + fn index(&self, index: Idx) -> &Self::Output { > + BStr::from_bytes(&self.0[index]) > + } > +} > + > /// Creates a new [`BStr`] from a string literal. > /// > /// `b_str!` converts the supplied string literal to byte string, so non-ASCII >
"Gary Guo" <gary@garyguo.net> writes: > On Tue, 11 Feb 2025 16:57:36 +0100 > Andreas Hindborg <a.hindborg@kernel.org> wrote: > >> The `Index` implementation on `BStr` was lost when we switched `BStr` from >> a type alias of `[u8]` to a newtype. This patch adds back `Index` by >> implementing `Index` for `BStr` when `Index` would be implemented for >> `[u8]`. >> >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org> >> --- >> rust/kernel/str.rs | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs >> index 002dcddf7c768..1eb945bed77d6 100644 >> --- a/rust/kernel/str.rs >> +++ b/rust/kernel/str.rs >> @@ -114,6 +114,17 @@ fn eq(&self, other: &Self) -> bool { >> } >> } >> >> +impl<Idx> Index<Idx> for BStr >> +where >> + Idx: core::slice::SliceIndex<[u8], Output = [u8]>, > > I think I'd prefer > > [T]: Index<Idx>, Is that equivalent? Best regards, Andreas Hindborg
On Tue, 11 Feb 2025 21:24:44 +0100 Andreas Hindborg <a.hindborg@kernel.org> wrote: > "Gary Guo" <gary@garyguo.net> writes: > > > On Tue, 11 Feb 2025 16:57:36 +0100 > > Andreas Hindborg <a.hindborg@kernel.org> wrote: > > > >> The `Index` implementation on `BStr` was lost when we switched `BStr` from > >> a type alias of `[u8]` to a newtype. This patch adds back `Index` by > >> implementing `Index` for `BStr` when `Index` would be implemented for > >> `[u8]`. > >> > >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org> > >> --- > >> rust/kernel/str.rs | 11 +++++++++++ > >> 1 file changed, 11 insertions(+) > >> > >> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs > >> index 002dcddf7c768..1eb945bed77d6 100644 > >> --- a/rust/kernel/str.rs > >> +++ b/rust/kernel/str.rs > >> @@ -114,6 +114,17 @@ fn eq(&self, other: &Self) -> bool { > >> } > >> } > >> > >> +impl<Idx> Index<Idx> for BStr > >> +where > >> + Idx: core::slice::SliceIndex<[u8], Output = [u8]>, > > > > I think I'd prefer > > > > [T]: Index<Idx>, > > Is that equivalent? Sorry, I meant `[u8]: Index<Idx>`. This makes more semantic sense that "what ever can index a byte slice, it can also index BStr". This is also how our CStr and the array primitive type implements its Index operation. They should be equivalent as libcore does impl<T, I> Index<I> for [T] where I: SliceIndex<[T]> { ... } Best, Gary > > > Best regards, > Andreas Hindborg > >
"Gary Guo" <gary@garyguo.net> writes: > On Tue, 11 Feb 2025 21:24:44 +0100 > Andreas Hindborg <a.hindborg@kernel.org> wrote: > >> "Gary Guo" <gary@garyguo.net> writes: >> >> > On Tue, 11 Feb 2025 16:57:36 +0100 >> > Andreas Hindborg <a.hindborg@kernel.org> wrote: >> > >> >> The `Index` implementation on `BStr` was lost when we switched `BStr` from >> >> a type alias of `[u8]` to a newtype. This patch adds back `Index` by >> >> implementing `Index` for `BStr` when `Index` would be implemented for >> >> `[u8]`. >> >> >> >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org> >> >> --- >> >> rust/kernel/str.rs | 11 +++++++++++ >> >> 1 file changed, 11 insertions(+) >> >> >> >> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs >> >> index 002dcddf7c768..1eb945bed77d6 100644 >> >> --- a/rust/kernel/str.rs >> >> +++ b/rust/kernel/str.rs >> >> @@ -114,6 +114,17 @@ fn eq(&self, other: &Self) -> bool { >> >> } >> >> } >> >> >> >> +impl<Idx> Index<Idx> for BStr >> >> +where >> >> + Idx: core::slice::SliceIndex<[u8], Output = [u8]>, >> > >> > I think I'd prefer >> > >> > [T]: Index<Idx>, >> >> Is that equivalent? > > Sorry, I meant `[u8]: Index<Idx>`. This makes more semantic sense that > "what ever can index a byte slice, it can also index BStr". This is > also how our CStr and the array primitive type implements its Index > operation. > > They should be equivalent as libcore does > > impl<T, I> Index<I> for [T] where I: SliceIndex<[T]> { ... } > What I originally wrote is `Idx` must be usable as an index for `[u8]`, yielding `[u8]` when indexing. The new one you suggest, I parse as `[u8]` should be indexable by `Idx`. This is less info. The compiler will also complain about the missing info: error[E0308]: mismatched types --> /home/aeh/src/linux-rust/module-params/rust/kernel/str.rs:141:26 | 141 | BStr::from_bytes(&self.0[index]) | ---------------- ^^^^^^^^^^^^^^ expected `&[u8]`, found `&<[u8] as Index<Idx>>::Output` | | | arguments to this function are incorrect | = note: expected reference `&[u8]` found reference `&<[u8] as Index<Idx>>::Output` = help: consider constraining the associated type `<[u8] as Index<Idx>>::Output` to `[u8]` If I constrain the output it's all fine again: [u8]: Index<Idx, Output = [u8]>, But as I said, I don't think it matters which direction we put this? Best regards, Andreas Hindborg
On 18.02.25 12:14, Andreas Hindborg wrote: > "Gary Guo" <gary@garyguo.net> writes: > >> On Tue, 11 Feb 2025 21:24:44 +0100 >> Andreas Hindborg <a.hindborg@kernel.org> wrote: >> >>> "Gary Guo" <gary@garyguo.net> writes: >>> >>>> On Tue, 11 Feb 2025 16:57:36 +0100 >>>> Andreas Hindborg <a.hindborg@kernel.org> wrote: >>>> >>>>> The `Index` implementation on `BStr` was lost when we switched `BStr` from >>>>> a type alias of `[u8]` to a newtype. This patch adds back `Index` by >>>>> implementing `Index` for `BStr` when `Index` would be implemented for >>>>> `[u8]`. >>>>> >>>>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org> >>>>> --- >>>>> rust/kernel/str.rs | 11 +++++++++++ >>>>> 1 file changed, 11 insertions(+) >>>>> >>>>> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs >>>>> index 002dcddf7c768..1eb945bed77d6 100644 >>>>> --- a/rust/kernel/str.rs >>>>> +++ b/rust/kernel/str.rs >>>>> @@ -114,6 +114,17 @@ fn eq(&self, other: &Self) -> bool { >>>>> } >>>>> } >>>>> >>>>> +impl<Idx> Index<Idx> for BStr >>>>> +where >>>>> + Idx: core::slice::SliceIndex<[u8], Output = [u8]>, >>>> >>>> I think I'd prefer >>>> >>>> [T]: Index<Idx>, >>> >>> Is that equivalent? >> >> Sorry, I meant `[u8]: Index<Idx>`. This makes more semantic sense that >> "what ever can index a byte slice, it can also index BStr". This is >> also how our CStr and the array primitive type implements its Index >> operation. >> >> They should be equivalent as libcore does >> >> impl<T, I> Index<I> for [T] where I: SliceIndex<[T]> { ... } >> > > What I originally wrote is `Idx` must be usable as an index for `[u8]`, > yielding `[u8]` when indexing. > > The new one you suggest, I parse as `[u8]` should be indexable by `Idx`. > This is less info. The compiler will also complain about the missing info: > > error[E0308]: mismatched types > --> /home/aeh/src/linux-rust/module-params/rust/kernel/str.rs:141:26 > | > 141 | BStr::from_bytes(&self.0[index]) > | ---------------- ^^^^^^^^^^^^^^ expected `&[u8]`, found `&<[u8] as Index<Idx>>::Output` > | | > | arguments to this function are incorrect > | > = note: expected reference `&[u8]` > found reference `&<[u8] as Index<Idx>>::Output` > = help: consider constraining the associated type `<[u8] as Index<Idx>>::Output` to `[u8]` > > If I constrain the output it's all fine again: > > [u8]: Index<Idx, Output = [u8]>, > > > But as I said, I don't think it matters which direction we put this? I think it's better to depend on `Index` compared to `SliceIndex`. --- Cheers, Benno
"Benno Lossin" <benno.lossin@proton.me> writes: > On 18.02.25 12:14, Andreas Hindborg wrote: >> "Gary Guo" <gary@garyguo.net> writes: >> >>> On Tue, 11 Feb 2025 21:24:44 +0100 >>> Andreas Hindborg <a.hindborg@kernel.org> wrote: >>> >>>> "Gary Guo" <gary@garyguo.net> writes: >>>> >>>>> On Tue, 11 Feb 2025 16:57:36 +0100 >>>>> Andreas Hindborg <a.hindborg@kernel.org> wrote: >>>>> >>>>>> The `Index` implementation on `BStr` was lost when we switched `BStr` from >>>>>> a type alias of `[u8]` to a newtype. This patch adds back `Index` by >>>>>> implementing `Index` for `BStr` when `Index` would be implemented for >>>>>> `[u8]`. >>>>>> >>>>>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org> >>>>>> --- >>>>>> rust/kernel/str.rs | 11 +++++++++++ >>>>>> 1 file changed, 11 insertions(+) >>>>>> >>>>>> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs >>>>>> index 002dcddf7c768..1eb945bed77d6 100644 >>>>>> --- a/rust/kernel/str.rs >>>>>> +++ b/rust/kernel/str.rs >>>>>> @@ -114,6 +114,17 @@ fn eq(&self, other: &Self) -> bool { >>>>>> } >>>>>> } >>>>>> >>>>>> +impl<Idx> Index<Idx> for BStr >>>>>> +where >>>>>> + Idx: core::slice::SliceIndex<[u8], Output = [u8]>, >>>>> >>>>> I think I'd prefer >>>>> >>>>> [T]: Index<Idx>, >>>> >>>> Is that equivalent? >>> >>> Sorry, I meant `[u8]: Index<Idx>`. This makes more semantic sense that >>> "what ever can index a byte slice, it can also index BStr". This is >>> also how our CStr and the array primitive type implements its Index >>> operation. >>> >>> They should be equivalent as libcore does >>> >>> impl<T, I> Index<I> for [T] where I: SliceIndex<[T]> { ... } >>> >> >> What I originally wrote is `Idx` must be usable as an index for `[u8]`, >> yielding `[u8]` when indexing. >> >> The new one you suggest, I parse as `[u8]` should be indexable by `Idx`. >> This is less info. The compiler will also complain about the missing info: >> >> error[E0308]: mismatched types >> --> /home/aeh/src/linux-rust/module-params/rust/kernel/str.rs:141:26 >> | >> 141 | BStr::from_bytes(&self.0[index]) >> | ---------------- ^^^^^^^^^^^^^^ expected `&[u8]`, found `&<[u8] as Index<Idx>>::Output` >> | | >> | arguments to this function are incorrect >> | >> = note: expected reference `&[u8]` >> found reference `&<[u8] as Index<Idx>>::Output` >> = help: consider constraining the associated type `<[u8] as Index<Idx>>::Output` to `[u8]` >> >> If I constrain the output it's all fine again: >> >> [u8]: Index<Idx, Output = [u8]>, >> >> >> But as I said, I don't think it matters which direction we put this? > > I think it's better to depend on `Index` compared to `SliceIndex`. I am curious for what reason? Best regards, Andreas Hindborg
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 002dcddf7c768..1eb945bed77d6 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -114,6 +114,17 @@ fn eq(&self, other: &Self) -> bool { } } +impl<Idx> Index<Idx> for BStr +where + Idx: core::slice::SliceIndex<[u8], Output = [u8]>, +{ + type Output = Self; + + fn index(&self, index: Idx) -> &Self::Output { + BStr::from_bytes(&self.0[index]) + } +} + /// Creates a new [`BStr`] from a string literal. /// /// `b_str!` converts the supplied string literal to byte string, so non-ASCII
The `Index` implementation on `BStr` was lost when we switched `BStr` from a type alias of `[u8]` to a newtype. This patch adds back `Index` by implementing `Index` for `BStr` when `Index` would be implemented for `[u8]`. Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org> --- rust/kernel/str.rs | 11 +++++++++++ 1 file changed, 11 insertions(+)