[v1,23/28] rust: std_vendor: add `dbg!` macro based on `std`'s one

Message ID 20221110164152.26136-24-ojeda@kernel.org
State New
Headers
Series Rust core additions |

Commit Message

Miguel Ojeda Nov. 10, 2022, 4:41 p.m. UTC
  From: Niklas Mohrin <dev@niklasmohrin.de>

The Rust standard library has a really handy macro, `dbg!` [1,2].
It prints the source location (filename and line) along with the raw
source code that is invoked with and the `Debug` representation
of the given expression, e.g.:

    let a = 2;
    let b = dbg!(a * 2) + 1;
    //      ^-- prints: [src/main.rs:2] a * 2 = 4
    assert_eq!(b, 5);

Port the macro over to the `kernel` crate, using `pr_info!`
instead of `eprintln!`, inside a new module called `std_vendor`.

Since the source code for the macro is taken from the standard
library source (with only minor adjustments), the new file is
licensed under `Apache 2.0 OR MIT`, just like the original [3,4].

Link: https://doc.rust-lang.org/std/macro.dbg.html [1]
Link: https://github.com/rust-lang/rust/blob/master/library/std/src/macros.rs#L212 [2]
Link: https://github.com/rust-lang/rust/blob/master/library/std/Cargo.toml [3]
Link: https://github.com/rust-lang/rust/blob/master/COPYRIGHT [4]
Signed-off-by: Niklas Mohrin <dev@niklasmohrin.de>
[Reworded, adapted for upstream and applied latest changes]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/kernel/lib.rs        |   2 +
 rust/kernel/prelude.rs    |   2 +-
 rust/kernel/std_vendor.rs | 160 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/std_vendor.rs
  

Comments

Boqun Feng Nov. 10, 2022, 6:01 p.m. UTC | #1
On Thu, Nov 10, 2022 at 05:41:35PM +0100, Miguel Ojeda wrote:
> From: Niklas Mohrin <dev@niklasmohrin.de>
> 
> The Rust standard library has a really handy macro, `dbg!` [1,2].
> It prints the source location (filename and line) along with the raw
> source code that is invoked with and the `Debug` representation
> of the given expression, e.g.:
> 
>     let a = 2;
>     let b = dbg!(a * 2) + 1;
>     //      ^-- prints: [src/main.rs:2] a * 2 = 4
>     assert_eq!(b, 5);
> 
> Port the macro over to the `kernel` crate, using `pr_info!`
> instead of `eprintln!`, inside a new module called `std_vendor`.
> 

I got confused when I saw `dbg!` uses `pr_info` instead of `pr_debug`,
but then I saw the discussion:

	https://github.com/Rust-for-Linux/linux/pull/483#discussion_r689881969

and I'm almost convinced ;-) Better add the gist of discussion into
comment/document/commit log? Users need to know when to use `dbg!` and
when to use `pr_debug!`, right?

Regards,
Boqun

> Since the source code for the macro is taken from the standard
> library source (with only minor adjustments), the new file is
> licensed under `Apache 2.0 OR MIT`, just like the original [3,4].
> 
> Link: https://doc.rust-lang.org/std/macro.dbg.html [1]
> Link: https://github.com/rust-lang/rust/blob/master/library/std/src/macros.rs#L212 [2]
> Link: https://github.com/rust-lang/rust/blob/master/library/std/Cargo.toml [3]
> Link: https://github.com/rust-lang/rust/blob/master/COPYRIGHT [4]
> Signed-off-by: Niklas Mohrin <dev@niklasmohrin.de>
> [Reworded, adapted for upstream and applied latest changes]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  rust/kernel/lib.rs        |   2 +
>  rust/kernel/prelude.rs    |   2 +-
>  rust/kernel/std_vendor.rs | 160 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 163 insertions(+), 1 deletion(-)
>  create mode 100644 rust/kernel/std_vendor.rs
> 
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index ffc6626a6d29..d6371c9c8453 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -26,6 +26,8 @@ mod allocator;
>  pub mod error;
>  pub mod prelude;
>  pub mod print;
> +#[doc(hidden)]
> +pub mod std_vendor;
>  pub mod str;
>  
>  #[doc(hidden)]
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index 89c2c9f4e7a7..345fc9075d1f 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -17,7 +17,7 @@ pub use alloc::{boxed::Box, vec::Vec};
>  
>  pub use macros::{module, vtable};
>  
> -pub use super::{pr_alert, pr_crit, pr_debug, pr_emerg, pr_err, pr_info, pr_notice, pr_warn};
> +pub use super::{dbg, pr_alert, pr_crit, pr_debug, pr_emerg, pr_err, pr_info, pr_notice, pr_warn};
>  
>  pub use super::error::{code::*, Error, Result};
>  
> diff --git a/rust/kernel/std_vendor.rs b/rust/kernel/std_vendor.rs
> new file mode 100644
> index 000000000000..da57b4e521f4
> --- /dev/null
> +++ b/rust/kernel/std_vendor.rs
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: Apache-2.0 OR MIT
> +
> +//! The contents of this file come from the Rust standard library, hosted in
> +//! the <https://github.com/rust-lang/rust> repository, licensed under
> +//! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
> +//! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
> +
> +/// [`std::dbg`], but using [`pr_info`] instead of [`eprintln`].
> +///
> +/// Prints and returns the value of a given expression for quick and dirty
> +/// debugging.
> +///
> +/// An example:
> +///
> +/// ```rust
> +/// let a = 2;
> +/// # #[allow(clippy::dbg_macro)]
> +/// let b = dbg!(a * 2) + 1;
> +/// //      ^-- prints: [src/main.rs:2] a * 2 = 4
> +/// assert_eq!(b, 5);
> +/// ```
> +///
> +/// The macro works by using the `Debug` implementation of the type of
> +/// the given expression to print the value with [`printk`] along with the
> +/// source location of the macro invocation as well as the source code
> +/// of the expression.
> +///
> +/// Invoking the macro on an expression moves and takes ownership of it
> +/// before returning the evaluated expression unchanged. If the type
> +/// of the expression does not implement `Copy` and you don't want
> +/// to give up ownership, you can instead borrow with `dbg!(&expr)`
> +/// for some expression `expr`.
> +///
> +/// The `dbg!` macro works exactly the same in release builds.
> +/// This is useful when debugging issues that only occur in release
> +/// builds or when debugging in release mode is significantly faster.
> +///
> +/// Note that the macro is intended as a debugging tool and therefore you
> +/// should avoid having uses of it in version control for long periods
> +/// (other than in tests and similar).
> +///
> +/// # Stability
> +///
> +/// The exact output printed by this macro should not be relied upon
> +/// and is subject to future changes.
> +///
> +/// # Further examples
> +///
> +/// With a method call:
> +///
> +/// ```rust
> +/// # #[allow(clippy::dbg_macro)]
> +/// fn foo(n: usize) {
> +///     if dbg!(n.checked_sub(4)).is_some() {
> +///         // ...
> +///     }
> +/// }
> +///
> +/// foo(3)
> +/// ```
> +///
> +/// This prints to the kernel log:
> +///
> +/// ```text,ignore
> +/// [src/main.rs:4] n.checked_sub(4) = None
> +/// ```
> +///
> +/// Naive factorial implementation:
> +///
> +/// ```rust
> +/// # #[allow(clippy::dbg_macro)]
> +/// # {
> +/// fn factorial(n: u32) -> u32 {
> +///     if dbg!(n <= 1) {
> +///         dbg!(1)
> +///     } else {
> +///         dbg!(n * factorial(n - 1))
> +///     }
> +/// }
> +///
> +/// dbg!(factorial(4));
> +/// # }
> +/// ```
> +///
> +/// This prints to the kernel log:
> +///
> +/// ```text,ignore
> +/// [src/main.rs:3] n <= 1 = false
> +/// [src/main.rs:3] n <= 1 = false
> +/// [src/main.rs:3] n <= 1 = false
> +/// [src/main.rs:3] n <= 1 = true
> +/// [src/main.rs:4] 1 = 1
> +/// [src/main.rs:5] n * factorial(n - 1) = 2
> +/// [src/main.rs:5] n * factorial(n - 1) = 6
> +/// [src/main.rs:5] n * factorial(n - 1) = 24
> +/// [src/main.rs:11] factorial(4) = 24
> +/// ```
> +///
> +/// The `dbg!(..)` macro moves the input:
> +///
> +/// ```ignore
> +/// /// A wrapper around `usize` which importantly is not Copyable.
> +/// #[derive(Debug)]
> +/// struct NoCopy(usize);
> +///
> +/// let a = NoCopy(42);
> +/// let _ = dbg!(a); // <-- `a` is moved here.
> +/// let _ = dbg!(a); // <-- `a` is moved again; error!
> +/// ```
> +///
> +/// You can also use `dbg!()` without a value to just print the
> +/// file and line whenever it's reached.
> +///
> +/// Finally, if you want to `dbg!(..)` multiple values, it will treat them as
> +/// a tuple (and return it, too):
> +///
> +/// ```
> +/// # #[allow(clippy::dbg_macro)]
> +/// assert_eq!(dbg!(1usize, 2u32), (1, 2));
> +/// ```
> +///
> +/// However, a single argument with a trailing comma will still not be treated
> +/// as a tuple, following the convention of ignoring trailing commas in macro
> +/// invocations. You can use a 1-tuple directly if you need one:
> +///
> +/// ```
> +/// # #[allow(clippy::dbg_macro)]
> +/// # {
> +/// assert_eq!(1, dbg!(1u32,)); // trailing comma ignored
> +/// assert_eq!((1,), dbg!((1u32,))); // 1-tuple
> +/// # }
> +/// ```
> +///
> +/// [`std::dbg`]: https://doc.rust-lang.org/std/macro.dbg.html
> +/// [`eprintln`]: https://doc.rust-lang.org/std/macro.eprintln.html
> +/// [`printk`]: https://www.kernel.org/doc/html/latest/core-api/printk-basics.html
> +#[macro_export]
> +macro_rules! dbg {
> +    // NOTE: We cannot use `concat!` to make a static string as a format argument
> +    // of `pr_info!` because `file!` could contain a `{` or
> +    // `$val` expression could be a block (`{ .. }`), in which case the `pr_info!`
> +    // will be malformed.
> +    () => {
> +        $crate::pr_info!("[{}:{}]\n", ::core::file!(), ::core::line!())
> +    };
> +    ($val:expr $(,)?) => {
> +        // Use of `match` here is intentional because it affects the lifetimes
> +        // of temporaries - https://stackoverflow.com/a/48732525/1063961
> +        match $val {
> +            tmp => {
> +                $crate::pr_info!("[{}:{}] {} = {:#?}\n",
> +                    ::core::file!(), ::core::line!(), ::core::stringify!($val), &tmp);
> +                tmp
> +            }
> +        }
> +    };
> +    ($($val:expr),+ $(,)?) => {
> +        ($($crate::dbg!($val)),+,)
> +    };
> +}
> -- 
> 2.38.1
>
  
Miguel Ojeda Nov. 10, 2022, 7:14 p.m. UTC | #2
On Thu, Nov 10, 2022 at 7:02 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> and I'm almost convinced ;-) Better add the gist of discussion into
> comment/document/commit log? Users need to know when to use `dbg!` and
> when to use `pr_debug!`, right?

The docs talk about it a bit:

    +/// Note that the macro is intended as a debugging tool and therefore you
    +/// should avoid having uses of it in version control for long periods
    +/// (other than in tests and similar).

That is the original wording from the standard library, but we can
definitely make the rules more concrete on our end with something
like:

    `dbg!` is intended as a temporary debugging tool to be used during
    development. Therefore, avoid committing `dbg!` macro invocations
    into the kernel tree.

    For debug output that is intended to be kept, use `pr_debug!` and
    similar facilities instead.

Cheers,
Miguel
  
Boqun Feng Nov. 10, 2022, 7:16 p.m. UTC | #3
On Thu, Nov 10, 2022 at 08:14:17PM +0100, Miguel Ojeda wrote:
> On Thu, Nov 10, 2022 at 7:02 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > and I'm almost convinced ;-) Better add the gist of discussion into
> > comment/document/commit log? Users need to know when to use `dbg!` and
> > when to use `pr_debug!`, right?
> 
> The docs talk about it a bit:
> 
>     +/// Note that the macro is intended as a debugging tool and therefore you
>     +/// should avoid having uses of it in version control for long periods
>     +/// (other than in tests and similar).
> 
> That is the original wording from the standard library, but we can
> definitely make the rules more concrete on our end with something

Yeah, having some kernel contexts is better ;-)

> like:
> 
>     `dbg!` is intended as a temporary debugging tool to be used during
>     development. Therefore, avoid committing `dbg!` macro invocations
>     into the kernel tree.
> 
>     For debug output that is intended to be kept, use `pr_debug!` and
>     similar facilities instead.
> 

Look good to me, thank you!

Regards,
Boqun

> Cheers,
> Miguel
  
Miguel Ojeda Nov. 10, 2022, 7:20 p.m. UTC | #4
On Thu, Nov 10, 2022 at 8:16 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> Yeah, having some kernel contexts is better ;-)

Agreed, and being able to tweak the docs is, after all, one of the
advantages of having a custom version in-tree anyway :)

Thanks a lot!

Cheers,
Miguel
  

Patch

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index ffc6626a6d29..d6371c9c8453 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -26,6 +26,8 @@  mod allocator;
 pub mod error;
 pub mod prelude;
 pub mod print;
+#[doc(hidden)]
+pub mod std_vendor;
 pub mod str;
 
 #[doc(hidden)]
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index 89c2c9f4e7a7..345fc9075d1f 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -17,7 +17,7 @@  pub use alloc::{boxed::Box, vec::Vec};
 
 pub use macros::{module, vtable};
 
-pub use super::{pr_alert, pr_crit, pr_debug, pr_emerg, pr_err, pr_info, pr_notice, pr_warn};
+pub use super::{dbg, pr_alert, pr_crit, pr_debug, pr_emerg, pr_err, pr_info, pr_notice, pr_warn};
 
 pub use super::error::{code::*, Error, Result};
 
diff --git a/rust/kernel/std_vendor.rs b/rust/kernel/std_vendor.rs
new file mode 100644
index 000000000000..da57b4e521f4
--- /dev/null
+++ b/rust/kernel/std_vendor.rs
@@ -0,0 +1,160 @@ 
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+
+//! The contents of this file come from the Rust standard library, hosted in
+//! the <https://github.com/rust-lang/rust> repository, licensed under
+//! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
+//! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
+
+/// [`std::dbg`], but using [`pr_info`] instead of [`eprintln`].
+///
+/// Prints and returns the value of a given expression for quick and dirty
+/// debugging.
+///
+/// An example:
+///
+/// ```rust
+/// let a = 2;
+/// # #[allow(clippy::dbg_macro)]
+/// let b = dbg!(a * 2) + 1;
+/// //      ^-- prints: [src/main.rs:2] a * 2 = 4
+/// assert_eq!(b, 5);
+/// ```
+///
+/// The macro works by using the `Debug` implementation of the type of
+/// the given expression to print the value with [`printk`] along with the
+/// source location of the macro invocation as well as the source code
+/// of the expression.
+///
+/// Invoking the macro on an expression moves and takes ownership of it
+/// before returning the evaluated expression unchanged. If the type
+/// of the expression does not implement `Copy` and you don't want
+/// to give up ownership, you can instead borrow with `dbg!(&expr)`
+/// for some expression `expr`.
+///
+/// The `dbg!` macro works exactly the same in release builds.
+/// This is useful when debugging issues that only occur in release
+/// builds or when debugging in release mode is significantly faster.
+///
+/// Note that the macro is intended as a debugging tool and therefore you
+/// should avoid having uses of it in version control for long periods
+/// (other than in tests and similar).
+///
+/// # Stability
+///
+/// The exact output printed by this macro should not be relied upon
+/// and is subject to future changes.
+///
+/// # Further examples
+///
+/// With a method call:
+///
+/// ```rust
+/// # #[allow(clippy::dbg_macro)]
+/// fn foo(n: usize) {
+///     if dbg!(n.checked_sub(4)).is_some() {
+///         // ...
+///     }
+/// }
+///
+/// foo(3)
+/// ```
+///
+/// This prints to the kernel log:
+///
+/// ```text,ignore
+/// [src/main.rs:4] n.checked_sub(4) = None
+/// ```
+///
+/// Naive factorial implementation:
+///
+/// ```rust
+/// # #[allow(clippy::dbg_macro)]
+/// # {
+/// fn factorial(n: u32) -> u32 {
+///     if dbg!(n <= 1) {
+///         dbg!(1)
+///     } else {
+///         dbg!(n * factorial(n - 1))
+///     }
+/// }
+///
+/// dbg!(factorial(4));
+/// # }
+/// ```
+///
+/// This prints to the kernel log:
+///
+/// ```text,ignore
+/// [src/main.rs:3] n <= 1 = false
+/// [src/main.rs:3] n <= 1 = false
+/// [src/main.rs:3] n <= 1 = false
+/// [src/main.rs:3] n <= 1 = true
+/// [src/main.rs:4] 1 = 1
+/// [src/main.rs:5] n * factorial(n - 1) = 2
+/// [src/main.rs:5] n * factorial(n - 1) = 6
+/// [src/main.rs:5] n * factorial(n - 1) = 24
+/// [src/main.rs:11] factorial(4) = 24
+/// ```
+///
+/// The `dbg!(..)` macro moves the input:
+///
+/// ```ignore
+/// /// A wrapper around `usize` which importantly is not Copyable.
+/// #[derive(Debug)]
+/// struct NoCopy(usize);
+///
+/// let a = NoCopy(42);
+/// let _ = dbg!(a); // <-- `a` is moved here.
+/// let _ = dbg!(a); // <-- `a` is moved again; error!
+/// ```
+///
+/// You can also use `dbg!()` without a value to just print the
+/// file and line whenever it's reached.
+///
+/// Finally, if you want to `dbg!(..)` multiple values, it will treat them as
+/// a tuple (and return it, too):
+///
+/// ```
+/// # #[allow(clippy::dbg_macro)]
+/// assert_eq!(dbg!(1usize, 2u32), (1, 2));
+/// ```
+///
+/// However, a single argument with a trailing comma will still not be treated
+/// as a tuple, following the convention of ignoring trailing commas in macro
+/// invocations. You can use a 1-tuple directly if you need one:
+///
+/// ```
+/// # #[allow(clippy::dbg_macro)]
+/// # {
+/// assert_eq!(1, dbg!(1u32,)); // trailing comma ignored
+/// assert_eq!((1,), dbg!((1u32,))); // 1-tuple
+/// # }
+/// ```
+///
+/// [`std::dbg`]: https://doc.rust-lang.org/std/macro.dbg.html
+/// [`eprintln`]: https://doc.rust-lang.org/std/macro.eprintln.html
+/// [`printk`]: https://www.kernel.org/doc/html/latest/core-api/printk-basics.html
+#[macro_export]
+macro_rules! dbg {
+    // NOTE: We cannot use `concat!` to make a static string as a format argument
+    // of `pr_info!` because `file!` could contain a `{` or
+    // `$val` expression could be a block (`{ .. }`), in which case the `pr_info!`
+    // will be malformed.
+    () => {
+        $crate::pr_info!("[{}:{}]\n", ::core::file!(), ::core::line!())
+    };
+    ($val:expr $(,)?) => {
+        // Use of `match` here is intentional because it affects the lifetimes
+        // of temporaries - https://stackoverflow.com/a/48732525/1063961
+        match $val {
+            tmp => {
+                $crate::pr_info!("[{}:{}] {} = {:#?}\n",
+                    ::core::file!(), ::core::line!(), ::core::stringify!($val), &tmp);
+                tmp
+            }
+        }
+    };
+    ($($val:expr),+ $(,)?) => {
+        ($($crate::dbg!($val)),+,)
+    };
+}