Support rooting arbitrary types

[jdm]

These changes support servo/servo#34194 by:

• ensuring crown does not complain about Rooted[Guard]<T> where T is annotated with must_root
• when dropping a rooted value, make clearing the value opt-in (originally introduced in servo/rust-mozjs@557a1ef)
• exposing the TraceableTrace trait needed to root arbitrary types

The GCMethods trait was being used in two contexts (Rooted<T> and Heap<T>) but the barrier operation is only relevant for Heap<T>. By splitting them out, we avoid the need to implement unused methods that make reviewers nervous.

[sagudev]

The GCMethods trait was being used in two contexts (Rooted and Heap) but the barrier operation is only relevant for Heap. By splitting them out, we avoid the need to implement unused methods that make reviewers nervous.

❤️

when dropping a rooted value, make clearing the value opt-in

Where exactly is this used, I cannot seems to find any.

exposing the TraceableTrace trait needed to root arbitrary types

Can we somehow unify TraceableTrace and Traceable? Something like this:

unsafe impl<T: Traceable> TraceableTrace for T {
   unsafe fn do_trace(&mut self, trc: *mut JSTracer) {
     <Self as Traceable>::trace(self, trc)
   }
 }

Also we definitely want to test rooting of arbitrary types

[jdm]

Where exactly is this used, I cannot seems to find any.

The Drop implementation for RootedGuard.

Can we somehow unify TraceableTrace and Traceable?

I've tried a bunch of different blanket impls like this but the compiler always complains that foreign traits have to be implemented on local types or T needs to be a type variable for a local type. Maybe it would be possible if Traceable and TraceableTrace were defined in the same crate?

[jdm]

Also that particular blanket impls leads to conflicts with duplicate RootKind implementations, because of another blanket impl that involves TraceableTrace.

[sagudev]

Where exactly is this used, I cannot seems to find any.

The Drop implementation for RootedGuard.

I would consider that as a consumer, I am asking is there any type that its impl actually returns None.

Maybe it would be possible if Traceable and TraceableTrace were defined in the same crate?

Yes I think that would be needed.

Also that particular blanket impls leads to conflicts with duplicate RootKind implementations, because of another blanket impl that involves TraceableTrace.

I think the problem is that we impl Traceable for some RootKind types, maybe we should remove them and start using TraceableTrace as main tracing trait (Traceable would only exists for easier impl of Tracing on custom types).

[sagudev]

No, it will need to go the other way, others should impl TraceableTrace and then we add just few additional impl for Traceable (those who have RootKind).

[jdm]

I would consider that as a consumer, I am asking is there any type that its impl actually returns None.

Right now, no. But it enables rooting arbitrary types in Servo without wrapping them in an Option.

[jdm]

I moved Traceable to mozjs-sys and was able to remove the duplication between TraceableTrace (now renamed to Rootable) and Traceable. I'm really pleased with how easy it is to make rooted! support arbitrary Rust types now!

[sagudev]

I moved Traceable to mozjs-sys and was able to remove the duplication between TraceableTrace (now renamed to Rootable) and Traceable. I'm really pleased with how easy it is to make rooted! support arbitrary Rust types now!

Not exactly what I had in mind, but I think it's better this way!

[jdm]

@sagudev Have you reviewed these changes but forgotten to mark them approved?

[sagudev]

@sagudev Have you reviewed these changes but forgotten to mark them approved?

I my mind I considered it OK, but companion PR does not work (might be sccache problem). Given the current state of mozjs being ahead of servo we should wait with landing this until stuff clears out.

[jdm]

Got it!

[sagudev]

We can rebase this and companion PR and hope for the best.