rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [v2] rust: introduce sfile macro for succinct code tracing
@ 2025-06-03 19:54 Timur Tabi
  2025-06-03 22:53 ` Miguel Ojeda
  2025-06-04  9:00 ` Alice Ryhl
  0 siblings, 2 replies; 7+ messages in thread
From: Timur Tabi @ 2025-06-03 19:54 UTC (permalink / raw)
  To: Miguel Ojeda, lossin, Alice Ryhl, Danilo Krummrich,
	rust-for-linux

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 apparently is not completely a const operation.

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

diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index cf4714242e14..1fd1497af887 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -414,3 +414,66 @@ macro_rules! pr_cont (
         $crate::print_macro!($crate::print::format_strings::CONT, true, $($arg)*)
     )
 );
+
+/// 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
+///
+/// ```
+/// pr_err!("{}:{}\n", sfile!(), line!());
+/// ```
+#[macro_export]
+macro_rules! sfile {
+    () => {{
+        const FILE: &str = file!();
+        assert!(FILE.is_ascii()); // .as_bytes() does not support non-ascii filenames
+
+        // Return the index of the last occurrence of `needle` in `haystack`,
+        // or zero if not found.  We can't use rfind() because it's not const (yet).
+        // Avoiding rfind() allows this macro to be evaluated at compile time.
+        const fn find_last_or_zero(haystack: &str, needle: char) -> usize {
+            let bytes = haystack.as_bytes();
+            let mut i = haystack.len();
+            while i > 0 {
+                i -= 1;
+                if bytes[i] == needle as u8 {
+                    return i;
+                }
+            }
+            0
+        }
+
+        // Return the index of the last occurrence of `needle` in `haystack`,
+        // or the length of the string if not found.
+        const fn find_last_or_len(haystack: &str, needle: char) -> usize {
+            let len = haystack.len();
+            let bytes = haystack.as_bytes();
+            let mut i = len;
+            while i > 0 {
+                i -= 1;
+                if bytes[i] == needle as u8 {
+                    return i;
+                }
+            }
+            len
+        }
+        let start = find_last_or_zero(FILE, '/') + 1;
+        let end = find_last_or_len(FILE, '.');
+
+        // We use these unsafe functions because apparently indexing into
+        // a string using normal Rust methods is not completely const operation.
+
+        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) }
+    }};
+}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] [v2] rust: introduce sfile macro for succinct code tracing
  2025-06-03 19:54 [PATCH] [v2] rust: introduce sfile macro for succinct code tracing Timur Tabi
@ 2025-06-03 22:53 ` Miguel Ojeda
  2025-06-06 22:11   ` Timur Tabi
  2025-06-04  9:00 ` Alice Ryhl
  1 sibling, 1 reply; 7+ messages in thread
From: Miguel Ojeda @ 2025-06-03 22:53 UTC (permalink / raw)
  To: Timur Tabi
  Cc: Miguel Ojeda, lossin, Alice Ryhl, Danilo Krummrich,
	rust-for-linux

On Tue, Jun 3, 2025 at 9:55 PM Timur Tabi <ttabi@nvidia.com> wrote:
>
> +/// built-in file!() macro, which returns the full path of the current file.

This is missing a code span and it is still using a single sentence
for the "short title".

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

Examples are built and tested, so please make sure they work -- this
will likely fail to build, since `sfile!` is not imported nor in the
prelude.

> +        assert!(FILE.is_ascii()); // .as_bytes() does not support non-ascii filenames

We generally avoid comments after a statement, instead we typically
put them in the previous line.

Also: code span is missing, "ASCII", and we end them with periods.

> +        // Return the index of the last occurrence of `needle` in `haystack`,
> +        // or zero if not found.  We can't use rfind() because it's not const (yet).
> +        // Avoiding rfind() allows this macro to be evaluated at compile time.

The first sentence should be `///`, and the rest a comment. It also
lacks code spans.

> +        // We use these unsafe functions because apparently indexing into
> +        // a string using normal Rust methods is not completely const operation.

Hmm... I am confused about what this is trying to say. `rfind` is not
const, but that does not imply "indexing into a string" is not const
(what "indexing into a string" means here?)

Also, we should try to avoid "apparently". And what does "not
completely const" mean here?

By the way, please use the standard "[...]" for patch titles -- you
can pass e.g. `-v2` to Git for that. (And ideally please provide `--base` too).

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [v2] rust: introduce sfile macro for succinct code tracing
  2025-06-03 19:54 [PATCH] [v2] rust: introduce sfile macro for succinct code tracing Timur Tabi
  2025-06-03 22:53 ` Miguel Ojeda
@ 2025-06-04  9:00 ` Alice Ryhl
  2025-06-04 20:43   ` Timur Tabi
  1 sibling, 1 reply; 7+ messages in thread
From: Alice Ryhl @ 2025-06-04  9:00 UTC (permalink / raw)
  To: Timur Tabi; +Cc: Miguel Ojeda, lossin, Danilo Krummrich, rust-for-linux

On Tue, Jun 03, 2025 at 02:54:26PM -0500, 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 apparently is not completely a const operation.
> 
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>

What is the use-case for this macro?

>  rust/kernel/print.rs | 63 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
> index cf4714242e14..1fd1497af887 100644
> --- a/rust/kernel/print.rs
> +++ b/rust/kernel/print.rs
> @@ -414,3 +414,66 @@ macro_rules! pr_cont (
>          $crate::print_macro!($crate::print::format_strings::CONT, true, $($arg)*)
>      )
>  );
> +
> +/// 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
> +///
> +/// ```
> +/// pr_err!("{}:{}\n", sfile!(), line!());
> +/// ```
> +#[macro_export]
> +macro_rules! sfile {
> +    () => {{
> +        const FILE: &str = file!();
> +        assert!(FILE.is_ascii()); // .as_bytes() does not support non-ascii filenames
> +
> +        // Return the index of the last occurrence of `needle` in `haystack`,
> +        // or zero if not found.  We can't use rfind() because it's not const (yet).
> +        // Avoiding rfind() allows this macro to be evaluated at compile time.
> +        const fn find_last_or_zero(haystack: &str, needle: char) -> usize {
> +            let bytes = haystack.as_bytes();
> +            let mut i = haystack.len();
> +            while i > 0 {
> +                i -= 1;
> +                if bytes[i] == needle as u8 {
> +                    return i;
> +                }
> +            }
> +            0
> +        }
> +
> +        // Return the index of the last occurrence of `needle` in `haystack`,
> +        // or the length of the string if not found.
> +        const fn find_last_or_len(haystack: &str, needle: char) -> usize {
> +            let len = haystack.len();
> +            let bytes = haystack.as_bytes();
> +            let mut i = len;
> +            while i > 0 {
> +                i -= 1;
> +                if bytes[i] == needle as u8 {
> +                    return i;
> +                }
> +            }
> +            len
> +        }
> +        let start = find_last_or_zero(FILE, '/') + 1;

Is plus one really correct when this returns zero?

> +        let end = find_last_or_len(FILE, '.');

I'm thinking that you should only have one method here returning Option,
and that the two calls should match on the Option.

let start = match find_last(FILE, '/') {
    Some(slash) => slash + 1,
    None => 0,
};
let end = match find_last(FILE, '.') {
    Some(dot) => dot,
    None => FILE.len(),
};

> +
> +        // We use these unsafe functions because apparently indexing into
> +        // a string using normal Rust methods is not completely const operation.
> +
> +        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) }

Instead of this, I'm thinking you could have a const str slicing method.
You can use is_char_boundary() asserts to get rid of the ASCII check.

Alice

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [v2] rust: introduce sfile macro for succinct code tracing
  2025-06-04  9:00 ` Alice Ryhl
@ 2025-06-04 20:43   ` Timur Tabi
  0 siblings, 0 replies; 7+ messages in thread
From: Timur Tabi @ 2025-06-04 20:43 UTC (permalink / raw)
  To: aliceryhl@google.com
  Cc: ojeda@kernel.org, dakr@kernel.org, lossin@kernel.org,
	rust-for-linux@vger.kernel.org

On Wed, 2025-06-04 at 09:00 +0000, Alice Ryhl wrote:
> On Tue, Jun 03, 2025 at 02:54:26PM -0500, 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 apparently is not completely a const operation.
> > 
> > Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> 
> What is the use-case for this macro?

That's been covered in other posts on this thread.  Short answer: it's like dbg!() but less noisy.

> > 
> > +        let start = find_last_or_zero(FILE, '/') + 1;
> 
> Is plus one really correct when this returns zero?

Oh, you're right!  I'll fix that in v3.  Thanks!

> > +        let end = find_last_or_len(FILE, '.');
> 
> I'm thinking that you should only have one method here returning Option,
> and that the two calls should match on the Option.
> 
> let start = match find_last(FILE, '/') {
>     Some(slash) => slash + 1,
>     None => 0,
> };
> let end = match find_last(FILE, '.') {
>     Some(dot) => dot,
>     None => FILE.len(),
> };

Hmmmm... that seems like a good idea.  I'll try it out, thanks.

> > +        // We use these unsafe functions because apparently indexing into
> > +        // a string using normal Rust methods is not completely const operation.
> > +
> > +        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) }
> 
> Instead of this, I'm thinking you could have a const str slicing method.
> You can use is_char_boundary() asserts to get rid of the ASCII check.

Um, can you spell that out a bit more?  I had to use AI to come up with this current monstrosity,
and I'm still not 100% sure how it works.  

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [v2] rust: introduce sfile macro for succinct code tracing
  2025-06-03 22:53 ` Miguel Ojeda
@ 2025-06-06 22:11   ` Timur Tabi
  2025-06-07  0:13     ` John Hubbard
  2025-06-07 10:17     ` Miguel Ojeda
  0 siblings, 2 replies; 7+ messages in thread
From: Timur Tabi @ 2025-06-06 22:11 UTC (permalink / raw)
  To: miguel.ojeda.sandonis@gmail.com
  Cc: ojeda@kernel.org, aliceryhl@google.com, lossin@kernel.org,
	dakr@kernel.org, rust-for-linux@vger.kernel.org

On Wed, 2025-06-04 at 00:53 +0200, Miguel Ojeda wrote:
> On Tue, Jun 3, 2025 at 9:55 PM Timur Tabi <ttabi@nvidia.com> wrote:
> > 
> > +/// built-in file!() macro, which returns the full path of the current file.
> 
> This is missing a code span and it is still using a single sentence
> for the "short title".
> 
> > +/// ```
> > +/// pr_err!("{}:{}\n", sfile!(), line!());
> > +/// ```
> 
> Examples are built and tested, so please make sure they work -- this
> will likely fail to build, since `sfile!` is not imported nor in the
> prelude.
> 
> > +        assert!(FILE.is_ascii()); // .as_bytes() does not support non-ascii filenames
> 
> We generally avoid comments after a statement, instead we typically
> put them in the previous line.
> 
> Also: code span is missing, "ASCII", and we end them with periods.
> 
> > +        // Return the index of the last occurrence of `needle` in `haystack`,
> > +        // or zero if not found.  We can't use rfind() because it's not const (yet).
> > +        // Avoiding rfind() allows this macro to be evaluated at compile time.
> 
> The first sentence should be `///`, and the rest a comment. It also
> lacks code spans.

Ok, I think I got it this time.

> > +        // We use these unsafe functions because apparently indexing into
> > +        // a string using normal Rust methods is not completely const operation.
> 
> Hmm... I am confused about what this is trying to say. `rfind` is not
> const, but that does not imply "indexing into a string" is not const
> (what "indexing into a string" means here?)
> 
> Also, we should try to avoid "apparently". And what does "not
> completely const" mean here?

Sorry, I didn't really understand your explanation, and I guess I failed to summarize it.  I don't
know what "not completely const" means, either.

I don't understand why, when I use "&FILE[start..]", this compiles as inline:

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

but this doesn't:

	const SFILE: &'static str = sfile!();

It seems to me that the second line is "more const" than the first line, if that makes any sense.  

What I was trying to say is that I need to use all those crazy unsafe calls so that the second line
works, but I can get away with just "&FILE[start..]" if I only cared about the first line.

> 
> By the way, please use the standard "[...]" for patch titles -- you
> can pass e.g. `-v2` to Git for that. (And ideally please provide `--base` too).

You mean you don't like [PATCH][V2]?  I've been doing that for 20 years, and this is the first time
anyone's complained.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [v2] rust: introduce sfile macro for succinct code tracing
  2025-06-06 22:11   ` Timur Tabi
@ 2025-06-07  0:13     ` John Hubbard
  2025-06-07 10:17     ` Miguel Ojeda
  1 sibling, 0 replies; 7+ messages in thread
From: John Hubbard @ 2025-06-07  0:13 UTC (permalink / raw)
  To: Timur Tabi, miguel.ojeda.sandonis@gmail.com
  Cc: ojeda@kernel.org, aliceryhl@google.com, lossin@kernel.org,
	dakr@kernel.org, rust-for-linux@vger.kernel.org

On 6/6/25 3:11 PM, Timur Tabi wrote:
> On Wed, 2025-06-04 at 00:53 +0200, Miguel Ojeda wrote:
>> On Tue, Jun 3, 2025 at 9:55 PM Timur Tabi <ttabi@nvidia.com> wrote:
...>> By the way, please use the standard "[...]" for patch titles -- you
>> can pass e.g. `-v2` to Git for that. (And ideally please provide `--base` too).
> 

--base is quite helpful, please don't overlook that, too.

> You mean you don't like [PATCH][V2]?  I've been doing that for 20 years, and this is the first time
> anyone's complained.
 During those 20 years, the kernel community moved to a mostly git-based
workflow. And git's "-v2" is a neat little time saver that also ensures that
your patchsets look "standard" to...pretty much everyone else here.

It's less about what you have been doing, and more about what others
expect to see.


thanks,
-- 
John Hubbard


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [v2] rust: introduce sfile macro for succinct code tracing
  2025-06-06 22:11   ` Timur Tabi
  2025-06-07  0:13     ` John Hubbard
@ 2025-06-07 10:17     ` Miguel Ojeda
  1 sibling, 0 replies; 7+ messages in thread
From: Miguel Ojeda @ 2025-06-07 10:17 UTC (permalink / raw)
  To: Timur Tabi
  Cc: ojeda@kernel.org, aliceryhl@google.com, lossin@kernel.org,
	dakr@kernel.org, rust-for-linux@vger.kernel.org

On Sat, Jun 7, 2025 at 12:11 AM Timur Tabi <ttabi@nvidia.com> wrote:
>
> I don't understand why, when I use "&FILE[start..]", this compiles as inline:
>
>         pr_err!("{}:{}\n", sfile!(), line!());
>
> but this doesn't:
>
>         const SFILE: &'static str = sfile!();
>
> It seems to me that the second line is "more const" than the first line, if that makes any sense.

The second call is in a const context, the first is just a normal
line. So the second must be a constant expression, which is a big
restriction on what you can write there, but the first doesn't require
that.

Hmm... Perhaps you are thinking that the compiler (or optimizer)
should know that it all resolves to a constant, and thus it should
allow it anyway.

However, constant evaluation doesn't work like that, i.e. even if some
expressions may happen to be able to be evaluated at compile-time, it
doesn't make them constant expressions.

It is similar to how C++'s `constexpr` works, if you have done that.
The languages keep expanding what can be done within const contexts
over time, but it is not based on whether a given instance could be.

(One place where we do use "this particular case must be X" kind of
logic is `build_assert!`).

> You mean you don't like [PATCH][V2]?  I've been doing that for 20 years, and this is the first time
> anyone's complained.

Sorry, I should have said "ideally" there too, since I intended it
mostly as a suggestion like the one in parenthesis. Neither of those
are blockers, of course, and we regularly take different styles of
patches. The `--base` thing is more "important" (useful).

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-06-07 10:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-03 19:54 [PATCH] [v2] rust: introduce sfile macro for succinct code tracing Timur Tabi
2025-06-03 22:53 ` Miguel Ojeda
2025-06-06 22:11   ` Timur Tabi
2025-06-07  0:13     ` John Hubbard
2025-06-07 10:17     ` Miguel Ojeda
2025-06-04  9:00 ` Alice Ryhl
2025-06-04 20:43   ` Timur Tabi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).