Re: [PATCH] rust: add `assert_sync` function

[Christian Schrefl <chrisi.schrefl@gmail.com>]

On 07.06.25 8:11 PM, Benno Lossin wrote:
> On Sat Jun 7, 2025 at 5:54 PM CEST, Christian Schrefl wrote:
>> On 07.06.25 5:42 PM, Benno Lossin wrote:
>>> On Sat Jun 7, 2025 at 3:02 PM CEST, Christian Schrefl wrote:
>>>> - Add `assert_send` as well.
>>>
>>> Sounds like a good idea.
>>
>> Should I already add this in V2 for this series?
> 
> If you want to then sure, but we can also wait until we have a use-case.
> Also, let's finish the discussion about the macro idea below.
> 
>>>> +///     assert_sync::<i32>(); // Succeeds because `i32` is Sync
>>>> +///     // assert_sync::<NotThreadSafe>(); // Fails because `NotThreadSafe` is not `Sync`.
>>>
>>> Can you split this into two examples and mark the failing one with
>>> `compile_fail`?
>>
>> I've tried it with `compile_fail` and it didn't work, I think
>> that's not supported in (kernel) doc tests yet. 
> 
> Hmm, I thought that this worked... @Miguel any idea?
> 
>>> We also could provide a macro similar to [1].
>>>
>>> [1]: https://docs.rs/static_assertions/latest/static_assertions/
>>
>> You mean the `assert_impl_*!` macros?
> 
> Yes, but the others might also be useful from time to time.
> >> That might make sense, with macros we would not need to write
>> a const block to ensure its not executed at runtime (although
>> it's probably optimized out anyways).
> 
> It 100% will be optimized out.
> 
>> It would also mean that we won't need a assert for every Trait, which
>> seems nice. So a macro sounds pretty good to me.
> 
> It depends, the macro impl needs to define its own function, which might
> be inefficient if one uses it a lot. But there is no way to be generic
> over traits, so there is no other way.
> 
> Let's see what the others think.

The error messages in the macro are slightly worse:
error[E0277]: `*mut ()` cannot be shared between threads safely
   --> rust/kernel/compile_assert.rs:40:18
    |
40  | assert_impl_all!(NotThreadSafe: Sync); // Fails because `NotThreadSafe` is not `Sync`
    |                  ^^^^^^^^^^^^^ `*mut ()` cannot be shared between threads safely
    |
    = help: within `PhantomData<*mut ()>`, the trait `Sync` is not implemented for `*mut ()`, which is required by `PhantomData<*mut ()>: Sync`
note: required because it appears within the type `PhantomData<*mut ()>`
   --> /home/chrisi/.rustup/toolchains/1.78-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/marker.rs:740:12
    |
740 | pub struct PhantomData<T: ?Sized>;
    |            ^^^^^^^^^^^
note: required by a bound in `assert_impl`
   --> rust/kernel/compile_assert.rs:34:48
    |
34  |             const fn assert_impl<T: ?Sized $(+ $trait)+>() {}
    |                                                ^^^^^^ required by this bound in `assert_impl`
...
40  | assert_impl_all!(NotThreadSafe: Sync); // Fails because `NotThreadSafe` is not `Sync`
    | ------------------------------------- in this macro invocation
    = note: this error originates in the macro `assert_impl_all` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 1 previous error

compared to the function:

error[E0277]: `*mut ()` cannot be shared between threads safely
   --> rust/kernel/compile_assert.rs:28:31
    |
28  | const _: () = { assert_sync::<NotThreadSafe>() };
    |                               ^^^^^^^^^^^^^ `*mut ()` cannot be shared between threads safely
    |
    = help: within `PhantomData<*mut ()>`, the trait `Sync` is not implemented for `*mut ()`, which is required by `PhantomData<*mut ()>: Sync`
note: required because it appears within the type `PhantomData<*mut ()>`
   --> /home/chrisi/.rustup/toolchains/1.78-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/marker.rs:740:12
    |
740 | pub struct PhantomData<T: ?Sized>;
    |            ^^^^^^^^^^^
note: required by a bound in `assert_sync`
   --> rust/kernel/compile_assert.rs:26:38
    |
26  | pub const fn assert_sync<T: ?Sized + Sync>() {}
    |                                      ^^^^ required by this bound in `assert_sync`

I guess I'll keep it as a function for now.

Cheers
Christian