-
Notifications
You must be signed in to change notification settings - Fork 925
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
Remove EventLoopError::AlreadyRunning
#3425
Conversation
This is already prevented by the type-system, and as such it doesn't make sense to have an error case for this.
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.
From a quick view this makes sense. The EventLoop
cannot be mutably borrowed more than once, and there's no way to Copy
/Clone
one to get multiple mutable references either.
Bit late but I think those checks relate to driving the loop with If you were to initially start running a loop with pump_events and get past dispatching Instead of trying to handle that unusual / unexpected mixing of |
I don't think that it's an issue actually? Like you can probably start with pump events for one iteration, but then continue with |
At least it should kind of work now. |
Yeah I think it's just that you'd get ambiguous semantics to try and do something like that, e.g. considering that you probably won't get a Since it's a weird case, I think my preference at the time was to throw an error instead of having to think about what semantics to promise users. |
This is already prevented by the type-system, and as such it doesn't make sense to have an error case for this.
This was added by @rib in #2767, maybe you can shed some light on if this was an oversight added out of an abundance of caution, or if you think my reasoning is wrong here, and the error actually can happen?
CHANGELOG.md
if knowledge of this change could be valuable to users