Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

INIT constrains which types can be used to implement RawMutex #400

Open
agausmann opened this issue Sep 4, 2023 · 8 comments
Open

INIT constrains which types can be used to implement RawMutex #400

agausmann opened this issue Sep 4, 2023 · 8 comments

Comments

@agausmann
Copy link

agausmann commented Sep 4, 2023

I think the lock-api crate has a lot of potential in embedded, but right now it has a limitation that implementations of RawMutex must provide an INIT constant value.

Some MCUs have their own hardware synchronization primitives, but these are unique resources / peripherals that must be owned in order to use them, and ownership is generally handed to the application through a PAC, or "peripheral access crate". They naturally cannot be obtained in a const context, and they also cannot be cloned since they rely on ownership being unique.

Peripherals like rp2040_hal::sio::Spinlock would be perfect for implementing RawMutex, but it's impossible to create a value for INIT without some kind of wrapper type that provides an illegal state.

INIT only seems necessary for Mutex::new and similar constructors that do not explicitly accept a RawMutex parameter, so I think it would make sense to move it into a separate trait. The raw mutex can still be used with the const_new constructors; it would operate similar to constructors that accept an allocator argument (Box::new_in etc.).

@agausmann agausmann changed the title INIT requirement constrains which types can be used to implement RawMutex INIT constrains which types can be used to implement RawMutex Sep 4, 2023
@agausmann
Copy link
Author

agausmann commented Sep 4, 2023

Sorry, the rp2040 Spinlock is not the best example, I had an incorrect assumption about how that API works.

This may still be worth considering, I still think it would be good to allow raw mutexes that can't provide an INIT. But right now I don't have a good example where that is important.

@Amanieu
Copy link
Owner

Amanieu commented Sep 5, 2023

Duplicate of #117

The problem is that this would prevent Mutex::new from being a const fn, which is hugely useful when using Mutex in a static. My general recommendation is to, if necessary, perform lazy initialization in the RawMutex itself when it is first used.

@agausmann
Copy link
Author

If we have a trait ConstInit, then Mutex::new would just require an additional bound R: ConstInit. I don't see how that would make it non-const.

@Amanieu
Copy link
Owner

Amanieu commented Sep 5, 2023

I suppose that could work. I would be happy to accept a PR that split const initialization into a separate trait. You'll need to add a separate constructor method to build a Mutex from a given RawMutex though.

@agausmann
Copy link
Author

You'll need to add a separate constructor method to build a Mutex from a given RawMutex though.

That already exists as Mutex::const_new (not my favorite name, but not a big deal)

@Amanieu
Copy link
Owner

Amanieu commented Sep 6, 2023

const_new should be considered deprecated. Since Rust 1.61 Mutex::new is a const fn. I would like to keep that.

Mutexes that don't support const-initialization should be pretty rare, and should be exposed through a Mutex::from_raw constructor.

@nspin
Copy link
Contributor

nspin commented Jan 12, 2024

I also have a use for the lock_api crate in a low-level environment where, for certain RawMutex implementations, initialization cannot happen in a const context.

However, to my surprise, I've found that the following works, provided that <T as RawMutex>::INIT is never actually evaluated, e.g. via Mutex::new:

const INIT: Self = unimplemented!();

If using unimplemented!() like this is considered an acceptable pattern for downstream crates which wish to use lock_api in this way (I don't know one way or another), then perhaps all that's really necessary is just adding a function Mutex::from_raw with the same type and behavior as Mutex::const_new.

@Amanieu
Copy link
Owner

Amanieu commented Jan 17, 2024

I'm happy to accept a PR to add Mutex::from_raw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants