Rust for Linux List
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: <sashiko-reviews@lists.linux.dev>, "Gary Guo" <gary@garyguo.net>
Cc: <ojeda@kernel.org>, <rust-for-linux@vger.kernel.org>
Subject: Re: [PATCH 2/3] rust: kunit: use `line!()` inside `kunit_assert!`
Date: Tue, 16 Jun 2026 16:48:16 +0100	[thread overview]
Message-ID: <DJALBE5W3ZZ6.2NRHWTDZPOC41@garyguo.net> (raw)
In-Reply-To: <20260616153845.32B241F00A3A@smtp.kernel.org>

(Cc list)

On Tue Jun 16, 2026 at 4:38 PM BST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] Macro hygiene prevents `kunit_assert!` from using the local `line!` override, breaking line number reporting in doctest failures.
> - [Low] Compilation error E0013 due to referencing a `static` item within an inline `const` block in the `line!` macro override.
> - [Low] Compile-time unsigned integer underflow in the overridden `line!` macro when invoked before the main test body.
> --
>
> commit bb46da20c6822f21e525800928a1b682691c33c6
> Author: Gary Guo <gary@garyguo.net>
> Date:   Tue Jun 16 15:24:50 2026 +0000
>
>     rust: kunit: use line!() inside kunit_assert!
>
>     This commit simplifies the kunit_assert macro by removing the diff
>     parameter and using the line! macro instead. It updates rustdoc_test_gen
>     to override the line! macro to correct line numbers in tests.
>
>> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
>> index 157fd0634708f..58dfec66b4f69 100644
>> --- a/rust/kernel/kunit.rs
>> +++ b/rust/kernel/kunit.rs
>
> [ ... ]
>
>> @@ -67,7 +67,7 @@ macro_rules! kunit_assert {
>>  
>>              // Use file!() instead of ::core::file!() here so it can be overridden.
>>              static FILE: &'static $crate::str::CStr = $crate::c_str!(file!());
>> -            static LINE: i32 = ::core::line!() as i32 - $diff;
>> +            static LINE: i32 = line!() as i32;
>>              static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($condition));
>>  
>
> [Severity: Medium]
> Will this actually use the overridden line! macro from the test wrapper?
>
> Because kunit_assert! is a declarative macro defined here, wouldn't macro
> hygiene cause line!() to resolve to core::line! at this definition site?
>
> If so, it seems the local macro override in rustdoc_test_gen.rs would be
> ignored, and failing tests would report the generated source line numbers
> instead of the original ones.

Not sure where Gemini gets this hallucination from, with mixed site hygiene only
local variables, labels and `$crate` are resolved from def site. There's not
even a way for macro invocations to resolve from def site at all without using
macro 2.0 hygiene.

>
>> diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs
>> index 37f4877f52e2d..acc4debe85923 100644
>> --- a/scripts/rustdoc_test_gen.rs
>> +++ b/scripts/rustdoc_test_gen.rs
>
> [ ... ]
>
>> @@ -169,18 +169,26 @@ fn main() {
>>              r#"/// Generated `{name}` KUnit test case from a Rust documentation test.
>>  #[no_mangle]
>>  pub extern "C" fn {kunit_name}(__kunit_test: *mut ::kernel::bindings::kunit) {{
>> -    /// Overrides the usual [`file!`] macro with one that expands to the real path.
>> +    // Overrides the usual [`file!`] macro with one that expands to the real path.
>>      #[allow(unused)]
>>      macro_rules! file {{
>>          () => {{ "{real_path}" }}
>>      }}
>>  
>> +    // Overrides the usual [`line!`] macro with one that expands to the real line number.
>> +    #[allow(unused)]
>> +    macro_rules! line {{
>> +        // NOTE: This does not expand to a literal, but a constant expression.
>> +        // Therefore code that depends on `line!()` being overrideable needs special adjustment.
>> +        () => {{ const {{ ::core::line!() - __DOCTEST_ANCHOR + {line} }} }}
>> +    }}
>> +
>
> [Severity: Low]
> Does this code cause a compilation error if the macro is ever invoked?
>
> Since __DOCTEST_ANCHOR is defined as a static item below, referencing it
> inside an inline const block violates Rust rules against reading statics
> in a const context, which would trigger compilation error E0013.

Somehow Sashiko is still pretending that it is the compiler. I've checked
Sashiko GitHub repo and it has already contains prompt that instruct it to
assume code builds cleanly. Somehow this is not working?

>
> Also, since the subtraction evaluates left-to-right on unsigned integers,
> could this underflow?
>
> If line! is called before the main body, ::core::line!() is less than
> __DOCTEST_ANCHOR. This compile-time subtraction would fail.

Impossible to happen.

Best,
Gary

  parent reply	other threads:[~2026-06-16 15:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-16 15:24 [PATCH 0/3] rust: doctest: unify with other kunit tests Gary Guo
2026-06-16 15:24 ` [PATCH 1/3] rust: kunit: use `file!()` inside `kunit_assert!` Gary Guo
2026-06-16 15:24 ` [PATCH 2/3] rust: kunit: use `line!()` " Gary Guo
     [not found]   ` <20260616153845.32B241F00A3A@smtp.kernel.org>
2026-06-16 15:48     ` Gary Guo [this message]
2026-06-16 15:24 ` [PATCH 3/3] rust: doctest: generate Rust kunit test suites Gary Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DJALBE5W3ZZ6.2NRHWTDZPOC41@garyguo.net \
    --to=gary@garyguo.net \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox