Make DeviceId/WindowId::dummy() safe
#3784
Conversation
|
Looks like @madsmtm asked this a while back here. The verdict is that it isn't a breaking change. For me, the question is "is there any way in which calling DeviceId::dummy() can put the program in an invalid state"? As far as I know, there isn't. In fact I think it's guaranteed for most backends that |
There was a problem hiding this comment.
I suspect it was previously unsafe because the WindowId is storing a HWND on Windows, and may have been passing that to window functions in the past? Or is this still done somewhere? If so, then we might have to change that first.
In any case, this is approved for the AppKit/UIKit platforms.
Looks like @madsmtm asked this a while back
And got it put into the documentation 💪.
There was a problem hiding this comment.
It should be fine given that APIs relying on WindowId are unsafe, but it doesn't make a dummy constructor unsafe because it can not ensure validity anyway, so the whole object is safe to write but unsafe to use since it doesn't retain anything.
So, if the API relies on Device/Window id, because they represent something it should be unsafe on its own, but not the constructor to build such object, because you can not do anything bad when building since it's pod.
src/changelog/unreleased.md
Outdated
| @@ -65,6 +65,7 @@ changelog entry. | |||
| `ApplicationHandler::resumed/suspended()` are now only emitted by iOS and Web | |||
| and now signify actually resuming/suspending the application. | |||
| - Rename `platform::web::*ExtWebSys` to `*ExtWeb`. | |||
| - `DeviceId::dummy()` and `WindowId::dummy()` are now not `unsafe`. | |||
There was a problem hiding this comment.
Probably this is a bit better than double negation.
| - `DeviceId::dummy()` and `WindowId::dummy()` are now not `unsafe`. | |
| - Mark `DeviceId::dummy()` and `WindowId::dummy()` as safe. |
There was a problem hiding this comment.
I intentionally avoided the word "safe" because its not 100% clear that we are talking about removing Rust unsafe.
| - `DeviceId::dummy()` and `WindowId::dummy()` are now not `unsafe`. | |
| - `DeviceId::dummy()` and `WindowId::dummy()` are no longer marked `unsafe`. |
Doesn't remove the double negation, but I think using the verb "mark" makes it much easier to understand.
I'm sure this had some historical context, but none of these IDs are passed into anything anymore.
I included this in the Winit 0.30.4 milestone because this isn't a breaking change.
Let me know if this is somehow a bad idea.