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

Replace rental with ouroboros #44

Closed
wants to merge 1 commit into from
Closed

Conversation

est31
Copy link
Contributor

@est31 est31 commented May 12, 2021

rental is not maintained any more and users
are encouraged to switch to alternatives.

rental is not maintained any more and users
are encouraged to switch to alternatives.
@Voultapher
Copy link

Voultapher commented May 13, 2021

@est31 curious that we've had a similar idea, of going through rental's reverse dependencies and migrating them. I guess because self_cell is quite new you didn't know about it? It is more limited than ouroboros but has some nice advantages eg. scpwiki/wikijump#270 so that imo it's a better fit for many current users of rental and ouroboros

@est31
Copy link
Contributor Author

est31 commented May 13, 2021

@Voultapher good point about the less dependencies. IDK what the maintainers prefer.

Lol indeed I'm going through the reverse dependencies. I was mainly motivated by the forward compat warning. Note that beyond the projects on crates.io, there is also a lot of direct use of rental. Even if the reverse dependencies are all migrated there will still be many projects using it. Maybe grep.app can help with finding those projects.

@Voultapher
Copy link

Yes I've been thinking about grep.app too, but more along the lines of trying to find handwritten unsafe self referential code like it's suggested tokio does here https://internals.rust-lang.org/t/improving-self-referential-structs/4808/75, where often it's probably unsound in some way. Having implemented self_cell and getting it wrong a dozen times I can sing a song about how tricky this problem is.

@Voultapher
Copy link

Sorry if we are sidetracking this issue, feel free to shoot me an email [email protected] I'd like to coordinate our effort out of band.

@@ -139,8 +139,8 @@ impl<R: Read + Write + Unpin> ImapStream<R> {
});
match res {
Ok(response) => Ok(Some(response)),
Err(rental::RentalError(err, block)) => {
self.buffer.return_block(block);
Err((err, heads)) => {

Choose a reason for hiding this comment

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

@Voultapher returning the owner seems to be missing from self_cell

Choose a reason for hiding this comment

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

This is a bit out of context. I suspect you are looking into using self_cell instead of ouroboros and some feature is missing. Please elaborate.

Choose a reason for hiding this comment

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

Yes, in this case with self_cell you would just get err back, not block/heads

Looking at the source it seems like self_cell just drops everything inside the cell when an error occurs

@link2xt
Copy link
Contributor

link2xt commented Sep 12, 2021

Replaced with #52 since I cannot push to this branch. #52 is a rebased and fixed version of this.

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

Successfully merging this pull request may close these issues.

4 participants