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

[WIP] Experiment with a safe adoption API #47

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
TODO: revert me; stub docs to get build to compile without warnings f…
…or CI dry run
  • Loading branch information
lopopolo committed Jun 15, 2021
commit b81dd3437eff04987c96cd801949e5ded5191831
2 changes: 2 additions & 0 deletions src/adopt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub unsafe trait Adopt: sealed::Sealed {
/// The smart pointer's inner owned value.
type Inner;

/// TODO: document me!
fn adopt(this: &mut Self, other: &Self)
where
Self::Inner: Trace;
Expand Down Expand Up @@ -90,6 +91,7 @@ unsafe impl<T> Adopt for Rc<T> {
/// `T`.
type Inner = T;

/// TODO: document me!
fn adopt(this: &mut Self, other: &Self)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe adopt should return Result<(), &Self> where the error variant contains other if the adoption failed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe there is try_adopt and adopt which unwraps try_adopt.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The E type in Result should implement Error, otherwise it won't compose well with existing error-handling mechanisms, such as the ? sugar.
  2. try_adopt that returns a Result and adopt that unwraps the former is indeed a good, ergonomic API. The standard library does that a lot too, I think.
  3. Why would you want to return other in case of a failure, if it's a reference and the callee already has it anyway?

where
Self::Inner: Trace,
Expand Down
2 changes: 2 additions & 0 deletions src/trace.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::rc::Rc;

/// TODO: document me!
pub trait Trace: Sized {
/// TODO: document me!
fn yield_owned_rcs<F>(&self, mark: F)
where
F: for<'a> FnMut(&'a mut Rc<Self>);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if the higher ranked lifetime bound is correct here. Do we want to enforce that the lifetime of the yielded &mut Rc<T> is the same as &self? Removing the for<'a> would disallow non-static T that have &mut Rc<T> fields which would be a good thing.

Expand Down