8000 Arc requires T: Sized · Issue #85 · tokio-rs/loom · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
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.

Sign up for GitHub

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

Jump to bottom

Arc requires T: Sized #85

Closed
jonhoo opened this issue Oct 23, 2019 · 8 comments · Fixed by #226
Closed

Arc requires T: Sized #85

jonhoo opened this issue Oct 23, 2019 · 8 comments · Fixed by #226

Comments

@jonhoo
Copy link
Contributor
jonhoo commented Oct 23, 2019

std::sync::Arc<T> does not require T: Sized, where as loom's version does. This means that code that uses, say, Arc<dyn Fn> will not compile with loom, which is unfortunate. I believe we would need to move the T to the end of loom::Arc for this to work, though that also breaks the layout needed according to loom::Arc's comment:
https://github.com/carllerche/loom/blob/339dd48f57d3b9b786e44db4914d08882d7659bf/src/sync/arc.rs#L11-L17
@jonhoo
Copy link
Contributor Author
jonhoo commented Oct 23, 2019

I believe what is actually needed here is CoerceUnsized which is unstable for the time being (tracking issue).

@carllerche
Copy link
Member

Closing as there is no immediate action to take.

@WaffleLapkin
Copy link
Contributor

Can't loom store a HashMap<*const (), std::sync::Arc<rt::Arc>> in its runtime (Execution?), so Inner wouldn't need to store rt::Arc after T?

I've just tried to test it and this seems to work. At least cargo test in loom with this change succeeds.

@jonhoo
Copy link
Contributor Author
jonhoo commented Sep 10, 2021

Not sure — @carllerche would need to chime in there.

@WaffleLapkin
Copy link
Contributor

Should I make a draft PR?

@jonhoo
Copy link
Contributor Author
jonhoo commented Sep 11, 2021

@WaffleLapkin I think that'd be helpful, and then we can always iterate from there!

@WaffleLapkin WaffleLapkin mentioned this issue Sep 14, 2021
Merged
@WaffleLapkin
Copy link
Contributor

For the record: I've created #226 a while ago with the intent to fix it. I think works and even allows a type like Arc<dyn Read> to be used. The problem is: without CoerceUnsized or something similar it's impossible to actually create Arc<dyn Read>.

@WaffleLapkin
Copy link
Contributor

Though since #226 makes loom::sync::Arc<T> basically store std::sync::Arc<T>, we could probably add a from_std function, so users could coerce the std arc and then create loom arc from it?

hawkw pushed a commit that referenced this issue Apr 23, 2022
@WaffleLapkin
Add Arc::from_std without T: Sized bound (#226)
d0c1e71 
* Remove (implicit) `T: Sized` bound from `Arc`

* Remove `Inner` struct from `Arc` as it's not needed anymore

* Add `Arc::from_std`

This function allows creating `Arc<T>` where `T: !Sized` by unsizing the
std's `Arc` first.

Fixes #85
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove (implicit) T: Sized bound from Arc WaffleLapkin/loom
3 participants
@carllerche @jonhoo @WaffleLapkin
0