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

Make GraphicsContext::window_mut() an unsafe fn. #5

Merged
merged 1 commit into from
Aug 24, 2022

Conversation

kpreid
Copy link
Contributor

@kpreid kpreid commented May 26, 2022

As documented in the new safety comment, providing &mut access to an inner component about which there are consistency invariants is unsafe, because &mut is sufficient for a caller to completely replace the value (using assignment or std::mem::swap()).

Luckily, when softbuffer is used with winit, no &mut access is needed. However, other windowing libraries such as sdl2 do have &mut methods, so this method can't simply be removed. (I don't know which windowing libraries would in fact be compatible with softbuffer and thus relevant.)

This is a breaking change since it makes a previously safe function unsafe, and should not be published without a major version bump (i.e. to 0.2.0 or higher).

As documented in the new safety comment, providing `&mut` access to an
inner component about which there are consistency invariants is unsafe,
because `&mut` is sufficient for a caller to completely replace the
value (using assignment or `std::mem::swap()`).

Luckily, when `softbuffer` is used with `winit`, no `&mut` access is
needed. However, other windowing libraries such as `glfw` and `sdl2` do
have `&mut` methods, so this method can't simply be removed.

This is a breaking change since it makes a previously safe function
unsafe, and should not be published without a major version bump
(i.e. to `0.2.0` or higher).
@john01dav john01dav merged commit e217b5c into rust-windowing:master Aug 24, 2022
@john01dav
Copy link
Collaborator

Sorry for the delay. I had some employment contract issues with FOSS contributions, and they are now resolved.

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

Successfully merging this pull request may close these issues.

2 participants