MutableHandle::new should be pub(crate)

[gterzian]

mozjs/mozjs/src/gc/root.rs

Line 123 in 0081fc4

pub(crate) fn new(ptr: &'a T) -> Self {

is private to this crate, but

mozjs/mozjs/src/gc/root.rs

Line 177 in 0081fc4

pub fn new(ptr: &'a mut T) -> Self {

is not. I assume both should be private to this crate(In both cases one should not directly call new, but rather use set on a newly rooted undefined value?).

[jdm]

Yep, that looks like the right fix to me.

[simonwuelker]

I don't understand this issue and I don't understand the fix. Please explain it to me.

I assume both should be private to this crate(In both cases one should not directly call new, but rather use set on a newly rooted undefined value?).

How would you use MutableHandle::set to replace the constructor functions?

Is this about some soundness issue that I'm not seeing?

For context:
I have a function that takes an out-parameter T and initializes it. T is gc-managed, the function can cause a garbage collection. When calling it, I sometimes have a &mut T and sometimes a RootedGuard<T>. I can't give the function a &mut T, that would be unsound because of the gc. So I would like to pass it a MutableHandle and then use MutableHandle::as_mut when I actually want to modify the value, avoiding the gc issue by creating a short-lived borrow. But that's impossible now because MutableHandle::new is not public anymore.

edit: Actually I suppose calling the function while I have &mut T is already unsound. So I would need something like pin_project for handles...

[sagudev]

You probably want to root T, and then you can obtain mutable handle to pass it to function.

MutableHandle::new is similar to MutableHandle::fromMarkedLocation, that has this comment in mozilla:

This may be called only if the location of the T is guaranteed
to be marked (for some reason other than being a Rooted),
e.g., if it is guaranteed to be reachable from an implicit root.