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

trace should be unsafe fn(cb: impl FnMut(&Frame) -> ControlFlow) #611

Open
workingjubilee opened this issue Apr 11, 2024 · 2 comments
Open

Comments

@workingjubilee
Copy link
Contributor

workingjubilee commented Apr 11, 2024

The war on bool will continue.

And thus we should use ControlFlow::Break to indicate forward progress should be stopped in backtrace's $(($MAJOR+1)) version.

@workingjubilee workingjubilee changed the title trace in backtrace $(($MAJOR+1)) should use ControlFlow trace should be fn trace(cb: impl FnMut(&Frame) -> ControlFlow) Apr 11, 2024
@workingjubilee
Copy link
Contributor Author

Obviously this change is somewhat gratuitous, so if anyone has some strong opinions otherwise, I'm listening. However, it's really important for an unsafe interface to be very clear, so...

@workingjubilee workingjubilee changed the title trace should be fn trace(cb: impl FnMut(&Frame) -> ControlFlow) trace should be unsafe fn(cb: impl FnMut(&Frame) -> ControlFlow) Apr 11, 2024
@ChrisDenton
Copy link
Contributor

I do think this make sense to me. trace is indeed just a loop in fancy clothing so ControlFlow is the most correct type, Correctness is important and, as you say, that's especially true in unsafe code.

This may not be the most crucial change in the world but if we're looking at ways to improve the API then this should be one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants