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

Follow simplification of Vec::retain() (rust-lang/rust#91527) #210

Closed
wants to merge 1 commit into from

Conversation

niklasf
Copy link
Contributor

@niklasf niklasf commented Dec 23, 2021

Follows rust-lang/rust#91527. This is neutral in my go-to benchmarks, but it's probably good to keep the implementation in sync regardless.

Edit: Looks like miri does not like this, whereas it doesn't mind std. Reminds me of #174 (review). Will debug later.

@niklasf
Copy link
Contributor Author

niklasf commented Dec 23, 2021

Fixed by dereferencing the pointer only for f(unsafe { &mut *cur }). To be honest, I don't understand how std gets away with not doing it like that.

@AngelicosPhosphoros
Copy link

@bluss Can we merge that?
It would fix complains of miri about retain.
#221

@AngelicosPhosphoros
Copy link

AngelicosPhosphoros commented Jul 17, 2022

@niklasf
At this moment miri also don't like ArrayVec::drain and ArrayVec::retain.

P.S. How about adding my code from issue as test to ensure that it remains valid?

@niklasf
Copy link
Contributor Author

niklasf commented Apr 9, 2023

Looks like the stacked borrow violations have been addressed in a different way.

@niklasf niklasf closed this Apr 9, 2023
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.

None yet

2 participants