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

Is it possible for loom::sync::Mutex::new to be const available? #320

Closed
NobodyXu opened this issue Jul 28, 2023 · 7 comments
Closed

Is it possible for loom::sync::Mutex::new to be const available? #320

NobodyXu opened this issue Jul 28, 2023 · 7 comments

Comments

@NobodyXu
Copy link

This comes from tokio-rs/tokio#5885 (comment) :

I think it's better to do this in a separate PR and it will require quite some duplication since rust doesn't permit cfg_attr(not(all(loom, test)), const).

If loom::sync::Mutex::new is available in const then tokio can remove the bound cfg(not(all(loom, test))) and make all {Mutex, Notify, ..}::new available in const without having to define two different versions depending on cfg(not(all(loom, test))).

From loom::sync::Mutex source code:

#[derive(Debug)]
pub struct Mutex<T> {
    object: rt::Mutex,
    data: std::sync::Mutex<T>,
}

it seems that it's mainly rt::Mutex and the MSRV blocking the method from being const.

rt::Mutex relies on global STATE, so the only way to make it const is to use OnceCell to initialize it lazily.

@Darksonn
Copy link
Contributor

I don't think that would be possible. Loom primitives must be dropped and recreated between each run.

@NobodyXu
Copy link
Author

I don't think that would be possible. Loom primitives must be dropped and recreated between each run.

They will still get dropped and recreated during each run.

What I meant is to change rt::Mutex to this:

#[derive(Debug, Copy, Clone)]
pub(crate) struct Mutex {
    state: OnceCell<object::Ref<State>>,
}

@Darksonn
Copy link
Contributor

If its in a global, then you can't recreate it after it is first initialized?

@NobodyXu
Copy link
Author

If its in a global, then you can't recreate it after it is first initialized?

No it's not global, it's still a local variable, just that it uses OnceCell to initialize lazily as opposed to initialize eagerly.

That will allow rt::Mutex::new to be const.

@NobodyXu
Copy link
Author

If you are ok with bumping msrv to 1.63, adding once_cell as a new dependency, I can submit two PRs for this (one for MSRV and other one for making it const).

@Darksonn
Copy link
Contributor

I don't think its necessary for us to do this. Even if we don't use it in globals, making it const will make people think that using it in globals is possible, but it is incorrect and will not work.

We can duplicate the new methods instead.

@NobodyXu
Copy link
Author

I don't think its necessary for us to do this. Even if we don't use it in globals, making it const will make people think that using it in globals is possible, but it is incorrect and will not work.

We can duplicate the new methods instead.

Yeah I agree that duplicating new and have a const_new is better than marking new as const, although it will still have to use OnceCell to delay initialization to runtime instead of at compile-time.

@Darksonn Darksonn closed this as not planned Won't fix, can't repro, duplicate, stale Jul 28, 2023
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

2 participants