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

Arc and Weak should be #[repr(transparent)] #55311

Closed
SoniEx2 opened this issue Oct 24, 2018 · 4 comments
Closed

Arc and Weak should be #[repr(transparent)] #55311

SoniEx2 opened this issue Oct 24, 2018 · 4 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@SoniEx2
Copy link
Contributor

SoniEx2 commented Oct 24, 2018

I make these assumptions: https://docs.rs/eventbus/0.2.0/src/eventbus/lib.rs.html#155

And I won't stop making them, even if I technically can't rely on them. There's absolutely no reason why you can't compare an Arc and a Weak without upgrading.

@NeoLegends
Copy link

NeoLegends commented Dec 9, 2018

(ignoring the highly offensive comment in your code that suggests you don't even want help)

Why is temporarily upgrading for the comparison a problem? Did you run into measurable performance issues?

That said, I don't think they should be #[repr(transparent)], because that would expose too much implementation detail. A proper way to see if an Arc and a Weak point to the same thing is the better option here.

@jonas-schievink
Copy link
Contributor

Yeah, this could be much more easily solved by adding Weak::as_ptr, and possibly Arc/Rc::as_ptr (which you don't really need since those Deref already). Alternatively, adding all 3 missing combinations of Arc/Rc::ptr_eq would work too.

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Dec 9, 2018

If you have an &Arc and an &Weak, e.g. because the Arc is the owned variant and the Weak is in a larger datastructure like a HashMap or a Vec, you can avoid atomic operations if you can just compare the Arc and the Weak directly. I'm not sure when it's desired to avoid atomic operations but ppl tell me it's a good idea to avoid atomic operations whenever possible. (they also say that, when possible, Relaxed is better than Aquire/Release is better than SeqCst)

Newer versions of eventbus don't compare Arcs and Weaks. This may change in the future, tho.

@jonas-schievink jonas-schievink added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Jan 27, 2019
@Mark-Simulacrum
Copy link
Member

We provide as_raw/into_raw on Weak and Arc (Rc as well, I believe) through #60728.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants