-
Notifications
You must be signed in to change notification settings - Fork 30
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
Make RwLockReadGuard
covariant
#45
Make RwLockReadGuard
covariant
#45
Conversation
c835356
to
c3aa0f9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather if you split some of this code into a new module. The rwlock.rs
file is getting too big as it is; it would be nice to split it off into a separate file.
I'd also appreciate it if you split the RawRwLock
code and the covariance code into two separate commits. It would make it easier to review. See this for a guide.
It would also be nice to add a test (i.e. use function bounds to make sure it's actually covariant) and to resolve the merge conflict.
And refactor `RwLock` implementation into non-generic core and generic interface
c3aa0f9
to
6f385ba
Compare
Done.
Unfortunately this isn't really possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Looks good to me aside from the minor typo.
@Jules-Bertholet Are you planning on making any other changes? If not I'd like to push this along with #42 as a new version. |
I was planning on making a PR for owned Arc guards... |
And refactor
RwLock
implementation into non-generic core and generic interface, which should reduce binary sizes.Fixes #9