Fixed some #[must_root] errors and added debugging code.

components/script/dom/bindings/inheritance.rs

@@ -13,25 +13,26 @@ use std::mem;
 
 /// A trait to hold the cast functions of IDL interfaces that either derive
 /// or are derived from other interfaces.
+#[must_root]
 pub trait Castable: IDLInterface + DomObject + Sized {
     /// Check whether a DOM object implements one of its deriving interfaces.
-    fn is<T>(&self) -> bool
+    fn is<#[must_root] T>(&self) -> bool
         where T: DerivedFrom<Self>
     {
         let class = unsafe { get_dom_class(self.reflector().get_jsobject().get()).unwrap() };
         T::derives(class)
     }
 
     /// Cast a DOM object upwards to one of the interfaces it derives from.
-    fn upcast<T>(&self) -> &T
+    fn upcast<#[must_root] T>(&self) -> &T
         where T: Castable,
               Self: DerivedFrom<T>
     {
         unsafe { mem::transmute(self) }
     }
 
     /// Cast a DOM object downwards to one of the interfaces it might implement.
-    fn downcast<T>(&self) -> Option<&T>
+    fn downcast<#[must_root] T>(&self) -> Option<&T>
         where T: DerivedFrom<Self>
     {
         if self.is::<T>() {

components/script/dom/bindings/reflector.rs

@@ -13,7 +13,7 @@ use std::default::Default;
 
 /// Create the reflector for a new DOM object and yield ownership to the
 /// reflector.
-pub fn reflect_dom_object<T, U>(
+pub fn reflect_dom_object<#[must_root] T, #[must_root] U>(
         obj: Box<T>,
         global: &U,
         wrap_fn: unsafe fn(*mut JSContext, &GlobalScope, Box<T>) -> DomRoot<T>)
@@ -74,6 +74,7 @@ impl Reflector {
 }
 
 /// A trait to provide access to the `Reflector` for a DOM object.
+#[must_root]
 pub trait DomObject: 'static {
     /// Returns the receiver's reflector.
     fn reflector(&self) -> &Reflector;
@@ -91,6 +92,7 @@ impl DomObject for Reflector {
 }
 
 /// A trait to initialize the `Reflector` for a DOM object.
+#[must_root]
 pub trait MutDomObject: DomObject {
     /// Initializes the Reflector
     fn init_reflector(&mut self, obj: *mut JSObject);

components/script/dom/bindings/root.rs

@@ -309,20 +309,20 @@ impl<'root, T: RootedReference<'root> + 'root> RootedReference<'root> for Option
 /// on the stack, the `Dom<T>` can point to freed memory.
 ///
 /// This should only be used as a field in other DOM objects.
-#[must_root]
-pub struct Dom<T> {
+// #[must_root]
+pub struct Dom<#[must_root] T> {
     ptr: ptr::NonNull<T>,
 }
 
 // Dom<T> is similar to Rc<T>, in that it's not always clear how to avoid double-counting.
 // For now, we choose not to follow any such pointers.
-impl<T> MallocSizeOf for Dom<T> {
+impl<#[must_root] T> MallocSizeOf for Dom<T> {
     fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize {
         0
     }
 }
 
-impl<T> Dom<T> {
+impl<#[must_root] T> Dom<T> {
     /// Returns `LayoutDom<T>` containing the same pointer.
     pub unsafe fn to_layout(&self) -> LayoutDom<T> {
         debug_assert!(thread_state::get().is_layout());
@@ -332,7 +332,7 @@ impl<T> Dom<T> {
     }
 }
 
-impl<T: DomObject> Dom<T> {
+impl<#[must_root] T: DomObject> Dom<T> {
     /// Create a Dom<T> from a &T
     #[allow(unrooted_must_root)]
     pub fn from_ref(obj: &T) -> Dom<T> {
@@ -546,11 +546,11 @@ impl<T: DomObject + PartialEq> PartialEq<T> for MutDom<T> {
 /// on `Dom<T>`.
 #[must_root]
 #[derive(JSTraceable)]
-pub struct MutNullableDom<T: DomObject> {
+pub struct MutNullableDom<#[must_root] T: DomObject> {
     ptr: UnsafeCell<Option<Dom<T>>>,
 }
 
-impl<T: DomObject> MutNullableDom<T> {
+impl<#[must_root] T: DomObject> MutNullableDom<T> {
     /// Create a new `MutNullableDom`.
     pub fn new(initial: Option<&T>) -> MutNullableDom<T> {
         debug_assert!(thread_state::get().is_script());
@@ -608,23 +608,23 @@ impl<T: DomObject> MutNullableDom<T> {
     }
 }
 
-impl<T: DomObject> PartialEq for MutNullableDom<T> {
+impl<#[must_root] T: DomObject> PartialEq for MutNullableDom<T> {
     fn eq(&self, other: &Self) -> bool {
         unsafe {
             *self.ptr.get() == *other.ptr.get()
         }
     }
 }
 
-impl<'a, T: DomObject> PartialEq<Option<&'a T>> for MutNullableDom<T> {
+impl<'a, #[must_root] T: DomObject> PartialEq<Option<&'a T>> for MutNullableDom<T> {
     fn eq(&self, other: &Option<&T>) -> bool {
         unsafe {
             *self.ptr.get() == other.map(Dom::from_ref)
         }
     }
 }
 
-impl<T: DomObject> Default for MutNullableDom<T> {
+impl<#[must_root] T: DomObject> Default for MutNullableDom<T> {
     #[allow(unrooted_must_root)]
     fn default() -> MutNullableDom<T> {
         debug_assert!(thread_state::get().is_script());
@@ -634,7 +634,7 @@ impl<T: DomObject> Default for MutNullableDom<T> {
     }
 }
 
-impl<T: DomObject> MallocSizeOf for MutNullableDom<T> {
+impl<#[must_root] T: DomObject> MallocSizeOf for MutNullableDom<T> {
     fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize {
         // See comment on MallocSizeOf for Dom<T>.
         0

components/script/dom/bindings/trace.rs

@@ -118,6 +118,7 @@ use webrender_api::{DocumentId, ImageKey};
 use webvr_traits::WebVRGamepadHand;
 
 /// A trait to allow tracing (only) DOM objects.
+#[must_root]
 pub unsafe trait JSTraceable {
     /// Trace `self`.
     unsafe fn trace(&self, trc: *mut JSTracer);

components/script/dom/node.rs

@@ -2488,11 +2488,11 @@ impl NodeMethods for Node {
     }
 }
 
-pub fn document_from_node<T: DerivedFrom<Node> + DomObject>(derived: &T) -> DomRoot<Document> {
+pub fn document_from_node<#[must_root] T: DerivedFrom<Node> + DomObject>(derived: &T) -> DomRoot<Document> {
     derived.upcast().owner_doc()
 }
 
-pub fn window_from_node<T: DerivedFrom<Node> + DomObject>(derived: &T) -> DomRoot<Window> {
+pub fn window_from_node<#[must_root] T: DerivedFrom<Node> + DomObject>(derived: &T) -> DomRoot<Window> {
     let document = document_from_node(derived);
     DomRoot::from_ref(document.window())
 }

components/script/timers.rs

@@ -75,7 +75,7 @@ pub enum OneshotTimerCallback {
 }
 
 impl OneshotTimerCallback {
-    fn invoke<T: DomObject>(self, this: &T, js_timers: &JsTimers) {
+    fn invoke<#[must_root] T: DomObject>(self, this: &T, js_timers: &JsTimers) {
         match self {
             OneshotTimerCallback::XhrTimeout(callback) => callback.invoke(),
             OneshotTimerCallback::EventSourceTimeout(callback) => callback.invoke(),
@@ -483,7 +483,7 @@ fn clamp_duration(nesting_level: u32, unclamped: MsDuration) -> MsDuration {
 
 impl JsTimerTask {
     // see https://html.spec.whatwg.org/multipage/#timer-initialisation-steps
-    pub fn invoke<T: DomObject>(self, this: &T, timers: &JsTimers) {
+    pub fn invoke<#[must_root] T: DomObject>(self, this: &T, timers: &JsTimers) {
         // step 4.1 can be ignored, because we proactively prevent execution
         // of this task when its scheduled execution is canceled.
 
@@ -502,7 +502,7 @@ impl JsTimerTask {
             },
             InternalTimerCallback::FunctionTimerCallback(ref function, ref arguments) => {
                 let arguments = self.collect_heap_args(arguments);
-                let _ = function.Call_(this, arguments, Report);
+                let _ = function.Call_(this, arguments, Report);  // TODO this is of type T, I can't locate where function is defined (somewhere in bind gen?)
             },
         };
 

components/script_plugins/unrooted_must_root.rs

@@ -193,7 +193,7 @@ impl<'a, 'b, 'tcx> UnrootedCx<'a, 'b, 'tcx> {
         ret
     }
 
-    fn has_unrooted_generic_substs(&self, did: hir::def_id::DefId, substs: &ty::subst::Substs) -> bool {
+    fn has_unrooted_generic_substs(&self, did: hir::def_id::DefId, substs: &ty::subst::Substs, st: &mut String) -> bool {
         let cx = self.late_cx;
 
         if cx.tcx.has_attr(did, "allow_unrooted_interior") {
@@ -202,6 +202,11 @@ impl<'a, 'b, 'tcx> UnrootedCx<'a, 'b, 'tcx> {
         // TODO we'll see what's necessary and what is not
         else if match_def_path(cx, did, &["core", "cell", "Ref"])
              || match_def_path(cx, did, &["core", "cell", "RefMut"])
+             || match_def_path(cx, did, &["core", "mem", "size_of"])  // -- is ok?
+             || match_def_path(cx, did, &["malloc_size_of", "MallocSizeOf"])  // -- is ok?
+             || match_def_path(cx, did, &["core", "option", "{{impl}}"])  // -- is ok?
+             || match_def_path(cx, did, &["core", "option", "Option"])  // -- is ok? looks like misuse
+             || match_def_path(cx, did, &["core", "ops", "deref", "Deref"])  // -- is ok? or only the deref method?
              || match_def_path(cx, did, &["core", "slice", "Iter"])
              || match_def_path(cx, did, &["std", "collections", "hash", "map", "Entry"])
              || match_def_path(cx, did, &["std", "collections", "hash", "map", "OccupiedEntry"])
@@ -211,6 +216,10 @@ impl<'a, 'b, 'tcx> UnrootedCx<'a, 'b, 'tcx> {
             return false;
         }
 
+        let def_path = get_def_path(cx, did);
+        st.push_str(&def_path.join("::"));
+        st.push_str(&"\n");
+
         let generics = cx.tcx.generics_of(did);
         for ty_param_def in &generics.types {
             // If type has `#[must_root]`, then it is ok to
@@ -221,12 +230,15 @@ impl<'a, 'b, 'tcx> UnrootedCx<'a, 'b, 'tcx> {
 
             let arg_ty = substs.type_at(ty_param_def.index as usize);
             if self.is_unrooted_ty(arg_ty, false) {
+                st.push_str(&" > ");
+                st.push_str(&arg_ty.to_string());
                 return true;
             }
         }
 
+        st.push_str(&" par$$ ");
         match generics.parent {
-            Some(p_did) => self.has_unrooted_generic_substs(p_did, substs),
+            Some(p_did) => self.has_unrooted_generic_substs(p_did, substs, st),
             None => false,
         }
     }
@@ -251,11 +263,14 @@ impl<'a, 'b, 'tcx> MirVisitor<'tcx> for MirFnVisitor<'a, 'b, 'tcx> {
         let cx = ur_cx.late_cx;
         match decl.ty.sty {
             ty::TyAdt(adt_def, substs) => {
+                let mut type_info = String::new();
                 if adt_def.is_box() && self.in_new_function {
                     // Boxes of unrooted types are allowed in new functions.
                 }
-                else if ur_cx.has_unrooted_generic_substs(adt_def.did, substs) {
-                    cx.span_lint(UNROOTED_MUST_ROOT, decl.source_info.span, "ADT generic type must be rooted.")
+                else if ur_cx.has_unrooted_generic_substs(adt_def.did, substs, &mut type_info) {
+                    let mut msg = "ADT generic type must be rooted.\n".to_string();
+                    msg.push_str(&type_info);
+                    cx.span_lint(UNROOTED_MUST_ROOT, decl.source_info.span, &msg);
                 }
             },
             _ => {},
@@ -270,16 +285,19 @@ impl<'a, 'b, 'tcx> MirVisitor<'tcx> for MirFnVisitor<'a, 'b, 'tcx> {
         match constant.ty.sty {
             ty::TyFnDef(callee_def_id, callee_substs) => {
                 let def_path = get_def_path(cx, callee_def_id);
+                let mut type_info = String::new();
                 // TODO remove following commented code
                 // cx.span_lint(UNROOTED_MUST_ROOT, constant.span, &def_path.join("::")); // tmp auxiliary call
                 if self.in_new_function && (// match_def_path(cx, callee_def_id, &["alloc", "boxed", "{{impl}}", "new"]) ||
                   def_path.last().unwrap().starts_with("new_") || def_path.last().unwrap() == "new") {
                       // "new"-like (constructors) functions are allowed
                 }
-                else if ur_cx.has_unrooted_generic_substs(callee_def_id, callee_substs) {
+                else if ur_cx.has_unrooted_generic_substs(callee_def_id, callee_substs, &mut type_info) {
                     // TODO tmp code - delete later
-                    let mut msg = "Callee generic type must be rooted.".to_string();
-                    msg.push_str(&def_path.join("::"));
+                    let mut msg = "Callee generic type must be rooted.\n".to_string();
+                    //msg.push_str(&def_path.join("::"));
+                    //msg.push_str(&"\n");
+                    msg.push_str(&type_info);
                     cx.span_lint(UNROOTED_MUST_ROOT, constant.span, &msg);
                     //cx.span_lint(UNROOTED_MUST_ROOT, constant.span, "Callee generic type must be rooted.")
                 }

tests/unit/script_plugins/lib.rs

@@ -299,6 +299,31 @@ pub mod unrooted_must_root {
     */
     pub fn allow_impl_for_must_root() {}
 
+    /* * // don't remember what exactly i wanted to check
+    ```
+    #![feature(plugin)]
+    #![plugin(script_plugins)]
+
+    #[must_root] struct Foo(i32);
+
+    #[must_root] //-- this is needed!
+    trait Bar {
+        fn new_method(&self);// { }
+    }
+
+    impl<T> Bar for Box<T> {
+        fn new_method(&self) { }
+    }
+
+    fn new_sth() {
+        Box::new(Foo(3)).new_method();
+    }
+
+    fn main() {}
+    ```
+    */
+    //pub fn sth_allow_impl_for_must_root() {}
+
     /* *
     ```
     #![feature(plugin)]