[PATCH v3] rust: introduce sfile macro for succinct code tracing

[PATCH v3] rust: introduce sfile macro for succinct code tracing

[Timur Tabi]

Introduce the sfile (short file) macro that returns the stem of
the current source file filename.

Rust provides a file!() macro that is similar to C's __FILE__ predefined
macro.  Unlike __FILE__, however, file!() returns a full path, which is
klunky when used for debug traces such as

	pr_info!("{}:{}\n", file!(), line!());

sfile!() can be used in situations instead, to provide a more compact
print.  For example, if file!() returns "rust/kernel/print.rs", sfile!()
returns just "print".

The macro avoids str::rfind() because currently, that function is not
const.  The compiler emits a call to memrchr, even when called on string
literals.  Instead, the macro implements its own versions of rfind(),
allowing the compiler to generate the slice at compile time.

The macro also uses some unsafe functions in order to avoid indexing
into a str, which is necessary to fully support const contexts.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 rust/kernel/print.rs | 66 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index 9783d960a97a..5b0c36481e95 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -423,3 +423,69 @@ macro_rules! pr_cont (
         $crate::print_macro!($crate::print::format_strings::CONT, true, $($arg)*)
     )
 );
+/// Returns the index of the last occurrence of `needle` in `haystack`, or zero.
+///
+/// Identical to [`str::rfind()`], but unlike that function, this one is const.
+/// That is, the compiler can inline it when called with a string literal.
+#[inline]
+pub const fn rfind_const(haystack: &str, needle: char) -> Option<usize> {
+    let bytes = haystack.as_bytes();
+    let mut i = haystack.len();
+    while i > 0 {
+        i -= 1;
+        if bytes[i] == needle as u8 {
+            return Some(i);
+        }
+    }
+    None
+}
+
+/// Returns just the base filename of the current file.
+///
+/// This differs from the built-in [`file!()`] macro, which returns the full
+/// path of the current file.
+///
+/// Using the base filename gives you more succinct logging prints.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::sfile
+/// pr_err!("{}:{}\n", sfile!(), line!());
+/// ```
+#[macro_export]
+macro_rules! sfile {
+    () => {{
+        use kernel::print::rfind_const;
+
+        const FILE: &str = file!();
+
+        // [`.as_bytes()`] does not support non-ASCII filenames
+        assert!(FILE.is_ascii());
+
+        let start = match rfind_const(FILE, '/') {
+            Some(slash) => slash + 1,
+            None => 0,
+        };
+        let end = match rfind_const(FILE, '.') {
+            Some(dot) => dot,
+            None => FILE.len(),
+        };
+
+        // The following code the equivalent of &FILE[start..start+len],
+        // except that it allows for const contexts.  For example,
+        //   const SFILE: &'static str = sfile!();
+
+        let base_ptr: *const u8 = FILE.as_ptr();
+
+        // SAFETY: We know that `start` is <= the length of FILE, because FILE
+        // never ends in a slash.
+        let ptr: *const u8 = unsafe { base_ptr.add(start) };
+        // SAFETY: We also know that `end` < the length of FILE.
+        // If `end` < `start`, this will generate a compiler error.
+        let slice = unsafe { core::slice::from_raw_parts(ptr, end - start) };
+        // SAFETY: We know that the slice is valid UTF-8, because we checked
+        // that FILE is ASCII (via is_ascii() above).
+        unsafe { core::str::from_utf8_unchecked(slice) }
+    }};
+}

base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
prerequisite-patch-id: 4bfda16a333b9f00a139050b7c6875922961a15d
-- 
2.43.0

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[John Hubbard]

On 6/9/25 3:36 PM, Timur Tabi wrote:
> Introduce the sfile (short file) macro that returns the stem of
> the current source file filename.
> 
> Rust provides a file!() macro that is similar to C's __FILE__ predefined
> macro.  Unlike __FILE__, however, file!() returns a full path, which is
> klunky when used for debug traces such as
> 
> 	pr_info!("{}:{}\n", file!(), line!());
> 
> sfile!() can be used in situations instead, to provide a more compact
> print.  For example, if file!() returns "rust/kernel/print.rs", sfile!()
> returns just "print".
> 
> The macro avoids str::rfind() because currently, that function is not
> const.  The compiler emits a call to memrchr, even when called on string
> literals.  Instead, the macro implements its own versions of rfind(),
> allowing the compiler to generate the slice at compile time.
> 
> The macro also uses some unsafe functions in order to avoid indexing
> into a str, which is necessary to fully support const contexts.
> 
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/print.rs | 66 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
> 
> diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
> index 9783d960a97a..5b0c36481e95 100644
> --- a/rust/kernel/print.rs
> +++ b/rust/kernel/print.rs
> @@ -423,3 +423,69 @@ macro_rules! pr_cont (
>          $crate::print_macro!($crate::print::format_strings::CONT, true, $($arg)*)
>      )
>  );
> +/// Returns the index of the last occurrence of `needle` in `haystack`, or zero.
> +///
> +/// Identical to [`str::rfind()`], but unlike that function, this one is const.
> +/// That is, the compiler can inline it when called with a string literal.
> +#[inline]
> +pub const fn rfind_const(haystack: &str, needle: char) -> Option<usize> {
> +    let bytes = haystack.as_bytes();

Silly nit: "bytes" is just adding an unnecessary variable that also has
a non-informative name. Just delete it...

> +    let mut i = haystack.len();
> +    while i > 0 {
> +        i -= 1;
> +        if bytes[i] == needle as u8 {

...and write the above line accordingly:

	if haystack.as_bytes()[i] == needle as u8 {


Yes?


thanks,
-- 
John Hubbard

> +            return Some(i);
> +        }
> +    }
> +    None
> +}
> +
> +/// Returns just the base filename of the current file.
> +///
> +/// This differs from the built-in [`file!()`] macro, which returns the full
> +/// path of the current file.
> +///
> +/// Using the base filename gives you more succinct logging prints.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sfile
> +/// pr_err!("{}:{}\n", sfile!(), line!());
> +/// ```
> +#[macro_export]
> +macro_rules! sfile {
> +    () => {{
> +        use kernel::print::rfind_const;
> +
> +        const FILE: &str = file!();
> +
> +        // [`.as_bytes()`] does not support non-ASCII filenames
> +        assert!(FILE.is_ascii());
> +
> +        let start = match rfind_const(FILE, '/') {
> +            Some(slash) => slash + 1,
> +            None => 0,
> +        };
> +        let end = match rfind_const(FILE, '.') {
> +            Some(dot) => dot,
> +            None => FILE.len(),
> +        };
> +
> +        // The following code the equivalent of &FILE[start..start+len],
> +        // except that it allows for const contexts.  For example,
> +        //   const SFILE: &'static str = sfile!();
> +
> +        let base_ptr: *const u8 = FILE.as_ptr();
> +
> +        // SAFETY: We know that `start` is <= the length of FILE, because FILE
> +        // never ends in a slash.
> +        let ptr: *const u8 = unsafe { base_ptr.add(start) };
> +        // SAFETY: We also know that `end` < the length of FILE.
> +        // If `end` < `start`, this will generate a compiler error.
> +        let slice = unsafe { core::slice::from_raw_parts(ptr, end - start) };
> +        // SAFETY: We know that the slice is valid UTF-8, because we checked
> +        // that FILE is ASCII (via is_ascii() above).
> +        unsafe { core::str::from_utf8_unchecked(slice) }
> +    }};
> +}
> 
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> prerequisite-patch-id: 4bfda16a333b9f00a139050b7c6875922961a15d

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Timur Tabi]

On Mon, 2025-06-09 at 19:33 -0700, John Hubbard wrote:
> > +/// Returns the index of the last occurrence of `needle` in `haystack`, or zero.
> > +///
> > +/// Identical to [`str::rfind()`], but unlike that function, this one is const.
> > +/// That is, the compiler can inline it when called with a string literal.
> > +#[inline]
> > +pub const fn rfind_const(haystack: &str, needle: char) -> Option<usize> {
> > +    let bytes = haystack.as_bytes();
> 
> Silly nit: "bytes" is just adding an unnecessary variable that also has
> a non-informative name. Just delete it...
> 
> > +    let mut i = haystack.len();
> > +    while i > 0 {
> > +        i -= 1;
> > +        if bytes[i] == needle as u8 {
> 
> ...and write the above line accordingly:
> 
> 	if haystack.as_bytes()[i] == needle as u8 {
> 
> 
> Yes?

I find my version to be easier to read.  I like using temporary local variables to keep my code
cleaner.

If I post a v4, I can try to come up with a better name.

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Benno Lossin]

On Tue Jun 10, 2025 at 5:40 AM CEST, Timur Tabi wrote:
> On Mon, 2025-06-09 at 19:33 -0700, John Hubbard wrote:
>> > +/// Returns the index of the last occurrence of `needle` in `haystack`, or zero.
>> > +///
>> > +/// Identical to [`str::rfind()`], but unlike that function, this one is const.
>> > +/// That is, the compiler can inline it when called with a string literal.
>> > +#[inline]
>> > +pub const fn rfind_const(haystack: &str, needle: char) -> Option<usize> {
>> > +    let bytes = haystack.as_bytes();
>> 
>> Silly nit: "bytes" is just adding an unnecessary variable that also has
>> a non-informative name. Just delete it...
>> 
>> > +    let mut i = haystack.len();
>> > +    while i > 0 {
>> > +        i -= 1;
>> > +        if bytes[i] == needle as u8 {
>> 
>> ...and write the above line accordingly:
>> 
>> 	if haystack.as_bytes()[i] == needle as u8 {
>> 
>> 
>> Yes?
>
> I find my version to be easier to read.  I like using temporary local variables to keep my code
> cleaner.
>
> If I post a v4, I can try to come up with a better name.

You can just reuse haystack:

    let haystack = haystack.as_bytes();

---
Cheers,
Benno

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Benno Lossin]

On Tue Jun 10, 2025 at 12:36 AM CEST, Timur Tabi wrote:
> Introduce the sfile (short file) macro that returns the stem of
> the current source file filename.
>
> Rust provides a file!() macro that is similar to C's __FILE__ predefined
> macro.  Unlike __FILE__, however, file!() returns a full path, which is
> klunky when used for debug traces such as
>
> 	pr_info!("{}:{}\n", file!(), line!());
>
> sfile!() can be used in situations instead, to provide a more compact
> print.  For example, if file!() returns "rust/kernel/print.rs", sfile!()
> returns just "print".
>
> The macro avoids str::rfind() because currently, that function is not
> const.  The compiler emits a call to memrchr, even when called on string
> literals.  Instead, the macro implements its own versions of rfind(),
> allowing the compiler to generate the slice at compile time.
>
> The macro also uses some unsafe functions in order to avoid indexing
> into a str, which is necessary to fully support const contexts.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/print.rs | 66 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>
> diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
> index 9783d960a97a..5b0c36481e95 100644
> --- a/rust/kernel/print.rs
> +++ b/rust/kernel/print.rs
> @@ -423,3 +423,69 @@ macro_rules! pr_cont (
>          $crate::print_macro!($crate::print::format_strings::CONT, true, $($arg)*)
>      )
>  );
> +/// Returns the index of the last occurrence of `needle` in `haystack`, or zero.
> +///
> +/// Identical to [`str::rfind()`], but unlike that function, this one is const.
> +/// That is, the compiler can inline it when called with a string literal.

The last sentence is incorrect. The compiler can always inline the
function. `const` means that the function can be called from const
evaluation such as in constant or static definitions, const blocks etc.

I would just remove the second sentence.

Another difference between this and `str::rfind` is that `str::rfind`
takes an `impl Pattern` and not a char. Additionally, I presume that
`str::rfind` is more efficiently implemented (as it's more complex, see
[1], but I didn't benchmark the two versions :).

[1]: https://doc.rust-lang.org/src/core/str/pattern.rs.html#478

> +#[inline]
> +pub const fn rfind_const(haystack: &str, needle: char) -> Option<usize> {
> +    let bytes = haystack.as_bytes();
> +    let mut i = haystack.len();
> +    while i > 0 {
> +        i -= 1;
> +        if bytes[i] == needle as u8 {
> +            return Some(i);
> +        }
> +    }
> +    None
> +}

This function should not be in this module, `str.rs` is probably a
better place for it.

> +
> +/// Returns just the base filename of the current file.
> +///
> +/// This differs from the built-in [`file!()`] macro, which returns the full
> +/// path of the current file.
> +///
> +/// Using the base filename gives you more succinct logging prints.

I would remove this sentence or replace it with something like "Useful
for succinct logging purposes.".

> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::sfile
> +/// pr_err!("{}:{}\n", sfile!(), line!());
> +/// ```
> +#[macro_export]
> +macro_rules! sfile {
> +    () => {{
> +        use kernel::print::rfind_const;

Please don't use `use` in macros.

> +
> +        const FILE: &str = file!();

Instead use absolute paths: `::core::file!()`.

> +
> +        // [`.as_bytes()`] does not support non-ASCII filenames
> +        assert!(FILE.is_ascii());

Also here.

> +
> +        let start = match rfind_const(FILE, '/') {

And here `::kernel::print::rfind_const` (more below).

> +            Some(slash) => slash + 1,
> +            None => 0,
> +        };
> +        let end = match rfind_const(FILE, '.') {
> +            Some(dot) => dot,
> +            None => FILE.len(),
> +        };
> +
> +        // The following code the equivalent of &FILE[start..start+len],
> +        // except that it allows for const contexts.  For example,
> +        //   const SFILE: &'static str = sfile!();
> +
> +        let base_ptr: *const u8 = FILE.as_ptr();
> +
> +        // SAFETY: We know that `start` is <= the length of FILE, because FILE
> +        // never ends in a slash.

I don't think that we should rely on that fact for unsafe code.

There is another argument for `start <= length`: when `rfind_const`
returns `Some(res)`, then `res < length`.

> +        let ptr: *const u8 = unsafe { base_ptr.add(start) };
> +        // SAFETY: We also know that `end` < the length of FILE.
> +        // If `end` < `start`, this will generate a compiler error.

No it won't generate a compiler error if used like in the example, it
will panic at runtime.

Maybe we should make this macro return a const block?

---
Cheers,
Benno

> +        let slice = unsafe { core::slice::from_raw_parts(ptr, end - start) };
> +        // SAFETY: We know that the slice is valid UTF-8, because we checked
> +        // that FILE is ASCII (via is_ascii() above).
> +        unsafe { core::str::from_utf8_unchecked(slice) }
> +    }};
> +}
>
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> prerequisite-patch-id: 4bfda16a333b9f00a139050b7c6875922961a15d

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Timur Tabi]

On Tue, 2025-06-10 at 10:45 +0200, Benno Lossin wrote:
> Another difference between this and `str::rfind` is that `str::rfind`
> takes an `impl Pattern` and not a char. Additionally, I presume that
> `str::rfind` is more efficiently implemented (as it's more complex, see
> [1], but I didn't benchmark the two versions :).
> 
> [1]: https://doc.rust-lang.org/src/core/str/pattern.rs.html#478

I'll reword the comments.

> 
> > +#[inline]
> > +pub const fn rfind_const(haystack: &str, needle: char) -> Option<usize> {
> > +    let bytes = haystack.as_bytes();
> > +    let mut i = haystack.len();
> > +    while i > 0 {
> > +        i -= 1;
> > +        if bytes[i] == needle as u8 {
> > +            return Some(i);
> > +        }
> > +    }
> > +    None
> > +}
> 
> This function should not be in this module, `str.rs` is probably a
> better place for it.

If I move it into str.rs, and I tag it with #[inline], will it still be inlined when used in the
macro?  I can't figure out how to test that in godbolt.

> > +/// Returns just the base filename of the current file.
> > +///
> > +/// This differs from the built-in [`file!()`] macro, which returns the full
> > +/// path of the current file.
> > +///
> > +/// Using the base filename gives you more succinct logging prints.
> 
> I would remove this sentence or replace it with something like "Useful
> for succinct logging purposes.".

Ok.

> 
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// use kernel::sfile
> > +/// pr_err!("{}:{}\n", sfile!(), line!());
> > +/// ```
> > +#[macro_export]
> > +macro_rules! sfile {
> > +    () => {{
> > +        use kernel::print::rfind_const;
> 
> Please don't use `use` in macros.

After moving rfind_const, it turns out I don't need the `use` statement anymore, but I don't
understand why.

> > +
> > +        const FILE: &str = file!();
> 
> Instead use absolute paths: `::core::file!()`.
> 
> > +
> > +        // [`.as_bytes()`] does not support non-ASCII filenames
> > +        assert!(FILE.is_ascii());
> 
> Also here.

Like this?

        assert!(::core::FILE.is_ascii());

This compiles, but it seems wrong.

> 
> > +
> > +        let start = match rfind_const(FILE, '/') {
> 
> And here `::kernel::print::rfind_const` (more below).

Even though I don't need to do this? The code compiles fine as-is.

Isn't the whole point behind `use` statements to avoid doing this?

> > +            Some(slash) => slash + 1,
> > +            None => 0,
> > +        };
> > +        let end = match rfind_const(FILE, '.') {
> > +            Some(dot) => dot,
> > +            None => FILE.len(),
> > +        };
> > +
> > +        // The following code the equivalent of &FILE[start..start+len],
> > +        // except that it allows for const contexts.  For example,
> > +        //   const SFILE: &'static str = sfile!();
> > +
> > +        let base_ptr: *const u8 = FILE.as_ptr();
> > +
> > +        // SAFETY: We know that `start` is <= the length of FILE, because FILE
> > +        // never ends in a slash.
> 
> I don't think that we should rely on that fact for unsafe code.
> 
> There is another argument for `start <= length`: when `rfind_const`
> returns `Some(res)`, then `res < length`.

I think that comment is stale, since it was written based on the original code.  I'll fix it.

> > +        let ptr: *const u8 = unsafe { base_ptr.add(start) };
> > +        // SAFETY: We also know that `end` < the length of FILE.
> > +        // If `end` < `start`, this will generate a compiler error.
> 
> No it won't generate a compiler error if used like in the example, it
> will panic at runtime.

Well, it's suppose to generate the basename inline, so that there are no runtime calls.  Otherwise,
I would never have had to create rfind_const.  

	pr_err!("{}:{}\n", sfile!(), line!());

This is supposed to be inline.  How do I get the kernel build to show me the generated assembly
language?  All of my tests in godbolt show that this code is always inline and the compiler notices
bad values of start and end.

> 
> Maybe we should make this macro return a const block?

You mean like this:

        const BASENAME = unsafe { core::str::from_utf8_unchecked(slice) };
        BASENAME

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Miguel Ojeda]

On Fri, Jun 13, 2025 at 7:03 PM Timur Tabi <ttabi@nvidia.com> wrote:
>
> If I move it into str.rs, and I tag it with #[inline], will it still be inlined when used in the
> macro?  I can't figure out how to test that in godbolt.

The crate is the compilation (translation) unit, i.e. like (typically)
each `.c` in C, so the compiler can still "see everything", if that is
what you mean.

Even across the crate boundary the compiler may still decide to
inline, as if it were in a C header.

`#[inline]` is not a guarantee, though.

> After moving rfind_const, it turns out I don't need the `use` statement anymore, but I don't
> understand why.

It may be imported by chance from a `*` import -- you should
double-check, but anyway, please use the full path instead.

> Like this?
>
>        assert!(::core::FILE.is_ascii());
>
> This compiles, but it seems wrong.

That should not compile -- were you calling the macro?

> Even though I don't need to do this? The code compiles fine as-is.
>
> Isn't the whole point behind `use` statements to avoid doing this?

In macros, you are expanding into the caller's code, so you cannot
assume what the user may have imported.

So please use fully qualified paths, including starting with
`::kernel` (i.e. not `kernel`).

> Well, it's suppose to generate the basename inline, so that there are no runtime calls.  Otherwise,
> I would never have had to create rfind_const.
>
>         pr_err!("{}:{}\n", sfile!(), line!());
>
> This is supposed to be inline.  How do I get the kernel build to show me the generated assembly
> language?  All of my tests in godbolt show that this code is always inline and the compiler notices
> bad values of start and end.

Const evaluation and inlining are two separate things. I think you are
trying to say that since everything is supposed to be inline then the
compiler should fold everything, but that is not guaranteed.

When you have code like the one in your patch, e.g. the `let`s are not
in a const context, and thus it is just normal code. It may happen to
be all optimized out, or not.

And if we made a mistake, e.g. replacing `start` with `1000`, you will
see it panics at runtime due to overflow if debug assertions are
enabled -- not a compile error.

But if you use a const context, like the initializer of a `const` or
putting it inside a `const` block, then the compiler will have to
evaluate it at compile-time (which is why not everything can be
written there).

As for showing the generated assembly, the object files are "normal"
ones, i.e. the usual tools should work. You can also put the `pr_err!`
line in e.g. a sample and then doing `make ....
samples/rust/rust_minimal.s` (eventually we will support `.s` for e.g.
`kernel.o` and other crates too).

> You mean like this:
>
>         const BASENAME = unsafe { core::str::from_utf8_unchecked(slice) };
>         BASENAME

I think Benno meant just wrapping all your macro expanded code within
a `const` block instead of a normal block -- no need to declare an
intermediate.

I hope that helps!

Cheers,
Miguel

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Timur Tabi]

On Fri, 2025-06-13 at 19:57 +0200, Miguel Ojeda wrote:
> > Like this?
> > 
> >        assert!(::core::FILE.is_ascii());
> > 
> > This compiles, but it seems wrong.
> 
> That should not compile -- were you calling the macro?

You're right, somehow my code calling this macro got deleted and didn't notice.  Thanks.

> > You mean like this:
> > 
> >         const BASENAME = unsafe { core::str::from_utf8_unchecked(slice) };
> >         BASENAME
> 
> I think Benno meant just wrapping all your macro expanded code within
> a `const` block instead of a normal block -- no need to declare an
> intermediate.

I can't seem to get this to work.  How exactly do I wrap this in a const block?  Everything I've
tried just generates all sorts of compilation errors I don't understand.

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Timur Tabi]

On Fri, 2025-06-13 at 14:14 -0500, Timur Tabi wrote:
> > I think Benno meant just wrapping all your macro expanded code within
> > a `const` block instead of a normal block -- no need to declare an
> > intermediate.
> 
> I can't seem to get this to work.  How exactly do I wrap this in a const block?  Everything I've
> tried just generates all sorts of compilation errors I don't understand.

I figured this out, also:

macro_rules! sfile {
    () => {{
        const {
            const FILE: &str = ::core::file!();

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Timur Tabi]

On Tue, 2025-06-10 at 10:45 +0200, Benno Lossin wrote:
> > +
> > +        // [`.as_bytes()`] does not support non-ASCII filenames
> > +        assert!(FILE.is_ascii());
> 
> Also here.

I can't get this to work, either.  I've tried all sorts of things, none of them work:

        assert!(::str::is_ascii(FILE));
        assert!(::core::str::is_ascii(FILE));
        assert!(FILE.str::is_ascii());

I even asked an AI and it just gives me wrong answers.  Also, no other drivers in the kernel do
this:

$ grep -rIw is_ascii *
macros/module.rs:        assert!(val.is_ascii(), "Expected ASCII string");
macros/helpers.rs:    assert!(string.is_ascii(), "Expected ASCII string");

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Timur Tabi]

On Fri, 2025-06-13 at 14:32 -0500, Timur Tabi wrote:
> On Tue, 2025-06-10 at 10:45 +0200, Benno Lossin wrote:
> > > +
> > > +        // [`.as_bytes()`] does not support non-ASCII filenames
> > > +        assert!(FILE.is_ascii());
> > 
> > Also here.
> 
> I can't get this to work, either.  I've tried all sorts of things, none of them work:
> 
>         assert!(::str::is_ascii(FILE));
>         assert!(::core::str::is_ascii(FILE));
>         assert!(FILE.str::is_ascii());

Never mind, I finally got it to work

	assert!(str::is_ascii(FILE));

Is `str` not part of the `core` module in the kernel?  It seems there are slight differences in the
module hierarchy in kernel vs. regular Rust.  Is there a document that shows this?

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Benno Lossin]

On Fri Jun 13, 2025 at 10:06 PM CEST, Timur Tabi wrote:
> On Fri, 2025-06-13 at 14:32 -0500, Timur Tabi wrote:
>> On Tue, 2025-06-10 at 10:45 +0200, Benno Lossin wrote:
>> > > +
>> > > +        // [`.as_bytes()`] does not support non-ASCII filenames
>> > > +        assert!(FILE.is_ascii());
>> > 
>> > Also here.
>> 
>> I can't get this to work, either.  I've tried all sorts of things, none of them work:
>> 
>>         assert!(::str::is_ascii(FILE));
>>         assert!(::core::str::is_ascii(FILE));

This one should work (you still need to make the `assert` use a path
though).

>>         assert!(FILE.str::is_ascii());
>
> Never mind, I finally got it to work
>
> 	assert!(str::is_ascii(FILE));
>
> Is `str` not part of the `core` module in the kernel?  It seems there are slight differences in the
> module hierarchy in kernel vs. regular Rust.  Is there a document that shows this?

The kernel uses the usual `core` so `str` should exist. What's the error
that you get?

---
Cheers,
Benno

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Timur Tabi]

On Sat, 2025-06-14 at 19:08 +0200, Benno Lossin wrote:
> > 
> > Never mind, I finally got it to work
> > 
> > 	assert!(str::is_ascii(FILE));
> > 
> > Is `str` not part of the `core` module in the kernel?  It seems there are slight differences in
> > the
> > module hierarchy in kernel vs. regular Rust.  Is there a document that shows this?
> 
> The kernel uses the usual `core` so `str` should exist. What's the error
> that you get?

            assert!(core::str::is_ascii(FILE));

error[E0425]: cannot find function `is_ascii` in module `core::str`
  --> samples/rust/rust_minimal.rs:22:29
   |
22 |         pr_info!("{}:{}\n", sfile!(), line!());
   |                             ^^^^^^^^ not found in `core::str`
   |
   = note: this error originates in the macro `sfile` (in Nightly builds, run with -Z macro-
backtrace for more info)

Same error with ::core::str.  If I write "kernel::str", it says the same thing about "kernel::str".

After a little more trial an error, the compiler told me that the correct namespace is:

            assert!(core::primitive::str::is_ascii(FILE));

That works.  Do you want me to use this or just str::is_ascii?

Also, how do I add "-Z macro-backtrace" to the build?

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Miguel Ojeda]

On Mon, Jun 16, 2025 at 5:37 PM Timur Tabi <ttabi@nvidia.com> wrote:
>
> After a little more trial an error, the compiler told me that the correct namespace is:
>
>             assert!(core::primitive::str::is_ascii(FILE));

Yeah, that looks fine since it comes from here:

    https://doc.rust-lang.org/core/primitive.char.html#method.is_ascii

(Please use `::core` within macros.)

> That works.  Do you want me to use this or just str::is_ascii?

`str` could in principle also be broken, e.g.:

    https://godbolt.org/z/Ysxnb1WPP

> Also, how do I add "-Z macro-backtrace" to the build?

You can use e.g.:

    make ... KRUSTFLAGS=-Zmacro-backtrace

I hope that helps!

Cheers,
Miguel

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Timur Tabi]

On Tue, 2025-06-10 at 10:45 +0200, Benno Lossin wrote:
> > +            Some(slash) => slash + 1,
> > +            None => 0,
> > +        };
> > +        let end = match rfind_const(FILE, '.') {
> > +            Some(dot) => dot,
> > +            None => FILE.len(),
> > +        };
> > +
> > +        // The following code the equivalent of &FILE[start..start+len],
> > +        // except that it allows for const contexts.  For example,
> > +        //   const SFILE: &'static str = sfile!();
> > +
> > +        let base_ptr: *const u8 = FILE.as_ptr();
> > +
> > +        // SAFETY: We know that `start` is <= the length of FILE, because FILE
> > +        // never ends in a slash.
> 
> I don't think that we should rely on that fact for unsafe code.
> 
> There is another argument for `start <= length`: when `rfind_const`
> returns `Some(res)`, then `res < length`.

The problem is the +1:

    let start = match ::kernel::str::rfind_const(FILE, '/') {
        Some(slash) => slash + 1,
        None => 0,
    };

`start` is the first character *after* the slash.  If FILE ever did end in a slash, then `start`
would point past the end of the string.  That didn't seem to cause a problem when I tested it, but
technically my code just assumes it's true.

I could add another assertion:

            assert!(start < FILE.len());

If I set FILE to "HELLO\", this causes a compiler error:

178 |         pr_info!("{}:{}\n", sfile!(), line!());
    |                             ^^^^^^^^ the evaluated program panicked at 'assertion failed:
start < FILE.len()', drivers/gpu/nova-core/gpu.rs:178:29

or even:

            assert!(start < end);

This code is only ever supposed on be used on ASCII filenames, which can never end in a slash.

I could do something like this:

            let start = match ::kernel::str::rfind_const(FILE, '/') {
                Some(slash) if slash == FILE.len() => slash,
                Some(slash) => slash + 1,
                None => 0,
            };

But I don't think this is useful.  

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Benno Lossin]

On Sat Jun 14, 2025 at 12:46 AM CEST, Timur Tabi wrote:
> On Tue, 2025-06-10 at 10:45 +0200, Benno Lossin wrote:
>> > +            Some(slash) => slash + 1,
>> > +            None => 0,
>> > +        };
>> > +        let end = match rfind_const(FILE, '.') {
>> > +            Some(dot) => dot,
>> > +            None => FILE.len(),
>> > +        };
>> > +
>> > +        // The following code the equivalent of &FILE[start..start+len],
>> > +        // except that it allows for const contexts.  For example,
>> > +        //   const SFILE: &'static str = sfile!();
>> > +
>> > +        let base_ptr: *const u8 = FILE.as_ptr();
>> > +
>> > +        // SAFETY: We know that `start` is <= the length of FILE, because FILE
>> > +        // never ends in a slash.
>> 
>> I don't think that we should rely on that fact for unsafe code.
>> 
>> There is another argument for `start <= length`: when `rfind_const`
>> returns `Some(res)`, then `res < length`.
>
> The problem is the +1:
>
>     let start = match ::kernel::str::rfind_const(FILE, '/') {
>         Some(slash) => slash + 1,
>         None => 0,
>     };
>
> `start` is the first character *after* the slash.  If FILE ever did end in a slash, then `start`
> would point past the end of the string.  That didn't seem to cause a problem when I tested it, but
> technically my code just assumes it's true.
>
> I could add another assertion:
>
>             assert!(start < FILE.len());
>
> If I set FILE to "HELLO\", this causes a compiler error:
>
> 178 |         pr_info!("{}:{}\n", sfile!(), line!());
>     |                             ^^^^^^^^ the evaluated program panicked at 'assertion failed:
> start < FILE.len()', drivers/gpu/nova-core/gpu.rs:178:29
>
> or even:
>
>             assert!(start < end);
>
> This code is only ever supposed on be used on ASCII filenames, which can never end in a slash.
>
> I could do something like this:
>
>             let start = match ::kernel::str::rfind_const(FILE, '/') {
>                 Some(slash) if slash == FILE.len() => slash,
>                 Some(slash) => slash + 1,
>                 None => 0,
>             };
>
> But I don't think this is useful.  

Let's use an assert, since it shouldn't happen in practice. We need to
assert it though for the unsafe code to be correct.

---
Cheers,
Benno

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Miguel Ojeda]

On Tue, Jun 10, 2025 at 12:36 AM Timur Tabi <ttabi@nvidia.com> wrote:
>
> +/// Returns the index of the last occurrence of `needle` in `haystack`, or zero.

Please provide examples (which double as tests) for new functions, and
possibly tests, especially for this kind that can easily be tested.

> +/// That is, the compiler can inline it when called with a string literal.

That is not really what `const` means. I would suggest just removing
this sentence, since we shouldn't explain `const` in every `const fn`.

> +/// ```
> +/// use kernel::sfile
> +/// pr_err!("{}:{}\n", sfile!(), line!());
> +/// ```

This doesn't build, since it is missing a semicolon.

As I mentioned in v2, examples are built and ran -- please do so
before submitting a patch:

    https://docs.kernel.org/rust/testing.html

> prerequisite-patch-id: 4bfda16a333b9f00a139050b7c6875922961a15d

Which prerequisite?

Thanks!

Cheers,
Miguel

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Timur Tabi]

On Tue, 2025-06-10 at 10:47 +0200, Miguel Ojeda wrote:
> On Tue, Jun 10, 2025 at 12:36 AM Timur Tabi <ttabi@nvidia.com> wrote:
> > 
> > +/// Returns the index of the last occurrence of `needle` in `haystack`, or zero.
> 
> Please provide examples (which double as tests) for new functions, and
> possibly tests, especially for this kind that can easily be tested.
> 
> > +/// That is, the compiler can inline it when called with a string literal.
> 
> That is not really what `const` means. I would suggest just removing
> this sentence, since we shouldn't explain `const` in every `const fn`.
> 
> > +/// ```
> > +/// use kernel::sfile
> > +/// pr_err!("{}:{}\n", sfile!(), line!());
> > +/// ```
> 
> This doesn't build, since it is missing a semicolon.

Sorry, bad copy and paste.  I will fix all the above issues in v4.

> As I mentioned in v2, examples are built and ran -- please do so
> before submitting a patch:
> 
>     https://docs.kernel.org/rust/testing.html
> 
> > prerequisite-patch-id: 4bfda16a333b9f00a139050b7c6875922961a15d
> 
> Which prerequisite?

Ugh, sorry.  I have another previous commit in my tree that I use to assist in building the kernel.
I don't see any way with --base to ignore that commit and skip generating that line.

I'm also a little confused by this line because 4bfda16a333b9f00a139050b7c6875922961a15d doesn't
exist in my repo.

I don't know any way around that other than to manually delete that line in the patch file before
emailing it.  Creating a temporary branch without that commit just to generate this patch file is
overkill.

Re: [PATCH v3] rust: introduce sfile macro for succinct code tracing

[Miguel Ojeda]

On Tue, Jun 10, 2025 at 9:18 PM Timur Tabi <ttabi@nvidia.com> wrote:
>
> Ugh, sorry.  I have another previous commit in my tree that I use to assist in building the kernel.
> I don't see any way with --base to ignore that commit and skip generating that line.

No worries. You could use e.g. `--base HEAD^`.

> I'm also a little confused by this line because 4bfda16a333b9f00a139050b7c6875922961a15d doesn't
> exist in my repo.

That is definitely strange... eager/early gc run?

Cheers,
Miguel