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

Remove DerefMut impl for Mutable Handle #573

Merged
merged 2 commits into from
Mar 28, 2025

Conversation

gmorenz
Copy link
Contributor

@gmorenz gmorenz commented Mar 26, 2025

Removes the deref_mut impl, and replaces it with an unsafe as_mut method. As discussed in #569 (and other recent issues) deref_mut is unsafe since it aliases with the GC, and cannot be made safe or marked unsafe.

The accompanying servo change (servo/servo#36160) does not need the as_mut method, it feels like an appropriate escape hatch to include though.

It also includes a MutableHandle::<Option<_>>::take method, since like in #572 that's a safe method, and #572's servo change results in it being used on MutableHandle.

Independent from (the obviously related) #572 and the similar incoming PR for JS::MutableHandle. Doing these all at roughly the same time feels like the right move, since otherwise we might accidentally encourage someone to convert from one of these types another to access the DerefMut function instead of using actual alternatives.

Signed-off-by: Greg Morenz <greg-morenz@droid.cafe>
@gmorenz gmorenz changed the title Mutable handle deref mut Remove DerefMut impl for RootedGuard Mar 26, 2025
@gmorenz gmorenz marked this pull request as draft March 26, 2025 02:35
@gmorenz
Copy link
Contributor Author

gmorenz commented Mar 26, 2025

Oops, this is not actually independent from #572. #572 creates an instance where MutableHandle::<Option<_>>::take is called... I'll fix that like I fixed the many more take calls in #572 by adding a take helper function. (Updated so it is independent again)

@gmorenz gmorenz force-pushed the mutable_handle_deref_mut branch from 4fd1835 to 725b383 Compare March 26, 2025 03:25
@gmorenz gmorenz marked this pull request as ready for review March 26, 2025 03:40
@gmorenz gmorenz changed the title Remove DerefMut impl for RootedGuard Remove DerefMut impl for Mutable Handle Mar 26, 2025
Signed-off-by: Greg Morenz <greg-morenz@droid.cafe>
@gmorenz gmorenz force-pushed the mutable_handle_deref_mut branch from 8e087ad to b198191 Compare March 27, 2025 19:46
@sagudev sagudev requested a review from jdm March 27, 2025 20:36
@jdm jdm added this pull request to the merge queue Mar 28, 2025
Merged via the queue into servo:main with commit e4d4f9a Mar 28, 2025
27 checks passed
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.

3 participants