Make stmt part of unrooted_must_root handle type parameters (fixes #6651)

components/plugins/lib.rs

@@ -27,6 +27,7 @@ use rustc::plugin::Registry;
 use syntax::ext::base::*;
 
 use syntax::parse::token::intern;
+use syntax::feature_gate::AttributeType::Whitelisted;
 
 // Public for documentation to show up
 /// Handles the auto-deriving for `#[derive(JSTraceable)]`
@@ -49,10 +50,13 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_macro("to_lower", casing::expand_lower);
     reg.register_macro("to_upper", casing::expand_upper);
     reg.register_lint_pass(box lints::transmute_type::TransmutePass as LintPassObject);
-    reg.register_lint_pass(box lints::unrooted_must_root::UnrootedPass as LintPassObject);
+    reg.register_lint_pass(box lints::unrooted_must_root::UnrootedPass::new() as LintPassObject);
     reg.register_lint_pass(box lints::privatize::PrivatizePass as LintPassObject);
     reg.register_lint_pass(box lints::inheritance_integrity::InheritancePass as LintPassObject);
     reg.register_lint_pass(box lints::str_to_string::StrToStringPass as LintPassObject);
     reg.register_lint_pass(box lints::ban::BanPass as LintPassObject);
     reg.register_lint_pass(box tenacious::TenaciousPass as LintPassObject);
+    reg.register_attribute("must_root".to_string(), Whitelisted);
+    reg.register_attribute("servo_lang".to_string(), Whitelisted);
+    reg.register_attribute("allow_unrooted_interior".to_string(), Whitelisted);
 }

components/plugins/lints/unrooted_must_root.rs

@@ -7,7 +7,7 @@ use syntax::attr::AttrMetaMethods;
 use rustc::ast_map;
 use rustc::lint::{Context, LintPass, LintArray};
 use rustc::middle::{ty, def};
-use utils::unsafe_context;
+use utils::{match_def_path, unsafe_context};
 
 declare_lint!(UNROOTED_MUST_ROOT, Deny,
               "Warn and report usage of unrooted jsmanaged objects");
@@ -25,8 +25,17 @@ declare_lint!(UNROOTED_MUST_ROOT, Deny,
 ///
 /// This helps catch most situations where pointers like `JS<T>` are used in a way that they can be invalidated by a
 /// GC pass.
-pub struct UnrootedPass;
+pub struct UnrootedPass {
+    in_new_function: bool
+}
 
+impl UnrootedPass {
+    pub fn new() -> UnrootedPass {
+        UnrootedPass {
+            in_new_function: true
+        }
+    }
+}
 // Checks if a type has the #[must_root] annotation.
 // Unwraps pointers as well
 // TODO (#3874, sort of): unwrap other types like Vec/Option/HashMap/etc
@@ -90,7 +99,10 @@ impl LintPass for UnrootedPass {
                 block: &ast::Block, _span: codemap::Span, id: ast::NodeId) {
         match kind {
             visit::FkItemFn(i, _, _, _, _, _) |
-            visit::FkMethod(i, _, _) if i.as_str() == "new" || i.as_str() == "new_inherited" => {
+            visit::FkMethod(i, _, _) if i.as_str() == "new"
+                                        || i.as_str() == "new_inherited"
+                                        || i.as_str() == "new_initialized" => {
+                self.in_new_function = true;
                 return;
             },
             visit::FkItemFn(_, _, style, _, _, _) => match style {
@@ -99,6 +111,7 @@ impl LintPass for UnrootedPass {
             },
             _ => ()
         }
+        self.in_new_function = false;
 
         if unsafe_context(&cx.tcx.map, id) {
             return;
@@ -120,7 +133,6 @@ impl LintPass for UnrootedPass {
     // Expressions which return out of blocks eventually end up in a `let` or assignment
     // statement or a function return (which will be caught when it is used elsewhere)
     fn check_stmt(&mut self, cx: &Context, s: &ast::Stmt) {
-
         match s.node {
             ast::StmtDecl(_, id) |
             ast::StmtExpr(_, id) |
@@ -155,16 +167,32 @@ impl LintPass for UnrootedPass {
             _ => return
         };
 
-        let t = cx.tcx.expr_ty(&*expr);
+        let ty = cx.tcx.expr_ty(&*expr);
+        ty.maybe_walk(|t| {
             match t.sty {
                 ty::TyStruct(did, _) |
                 ty::TyEnum(did, _) => {
                     if cx.tcx.has_attr(did, "must_root") {
                         cx.span_lint(UNROOTED_MUST_ROOT, expr.span,
-                                 &format!("Expression of type {:?} must be rooted", t));
+                                     &format!("Expression of type {:?}  in type {:?} must be rooted", t, ty));
+                        false
+                    } else if cx.tcx.has_attr(did, "allow_unrooted_interior") {
+                        false
+                    } else if match_def_path(cx, did, &["core", "cell", "Ref"])
+                            || match_def_path(cx, did, &["core", "cell", "RefMut"]) {
+                            // Ref and RefMut are borrowed pointers, okay to hold unrooted stuff
+                            // since it will be rooted elsewhere
+                        false
+                    } else {
+                        true
                     }
+                },
+                ty::TyBox(..) if self.in_new_function => false, // box in new() is okay
+                ty::TyRef(..) => false, // don't recurse down &ptrs
+                ty::TyRawPtr(..) => false, // don't recurse down *ptrs
+                _ => true
             }
-            _ => {}
+        })
-        }
+
     }
 }

components/plugins/utils.rs

@@ -49,7 +49,11 @@ pub fn match_lang_ty(cx: &Context, ty: &Ty, value: &str) -> bool {
         _ => return false,
     };
 
-    cx.tcx.get_attrs(def_id).iter().any(|attr| {
+    match_lang_did(cx, def_id, value)
+}
+
+pub fn match_lang_did(cx: &Context, did: ast::DefId, value: &str) -> bool {
+    cx.tcx.get_attrs(did).iter().any(|attr| {
         match attr.node.value.node {
             ast::MetaNameValue(ref name, ref val) if &**name == "servo_lang" => {
                 match val.node {
@@ -88,3 +92,11 @@ pub fn unsafe_context(map: &ast_map::Map, id: ast::NodeId) -> bool {
                    // to be added.
     }
 }
+
+/// check if a DefId's path matches the given absolute type path
+/// usage e.g. with
+/// `match_def_path(cx, id, &["core", "option", "Option"])`
+pub fn match_def_path(cx: &Context, def_id: ast::DefId, path: &[&str]) -> bool {
+    cx.tcx.with_path(def_id, |iter| iter.map(|elem| elem.name())
+        .zip(path.iter()).all(|(nm, p)| &nm.as_str() == p))
+}

components/script/dom/bindings/js.rs

@@ -382,6 +382,7 @@ pub unsafe fn trace_roots(tracer: *mut JSTracer) {
 /// are additive, so this object's destruction will not invalidate other roots
 /// for the same JS value. `Root`s cannot outlive the associated
 /// `RootCollection` object.
+#[allow_unrooted_interior]
 pub struct Root<T: Reflectable> {
     /// Reference to rooted value that must not outlive this container
     ptr: NonZero<*const T>,

components/script/dom/bindings/refcounted.rs

@@ -57,6 +57,7 @@ unsafe impl Send for TrustedReference {}
 /// shared among tasks for use in asynchronous operations. The underlying
 /// DOM object is guaranteed to live at least as long as the last outstanding
 /// `Trusted<T>` instance.
+#[allow_unrooted_interior]
 pub struct Trusted<T: Reflectable> {
     /// A pointer to the Rust DOM object of type T, but void to allow
     /// sending `Trusted<T>` between tasks, regardless of T's sendability.

components/script/dom/bindings/trace.rs

@@ -455,6 +455,7 @@ impl<'a, T: JSTraceable> Drop for RootedTraceable<'a, T> {
 #[allow(unrooted_must_root)]
 #[no_move]
 #[derive(JSTraceable)]
+#[allow_unrooted_interior]
 pub struct RootedVec<T: JSTraceable + Reflectable> {
     v: Vec<T>
 }

components/script/dom/bindings/utils.rs

@@ -413,8 +413,7 @@ pub fn reflect_dom_object<T: Reflectable>
 }
 
 /// A struct to store a reference to the reflector of a DOM object.
-// Allowing unused_attribute because the lint sometimes doesn't run in order
+#[allow(raw_pointer_derive, unrooted_must_root)]
-#[allow(raw_pointer_derive, unrooted_must_root, unused_attributes)]
 #[must_root]
 #[servo_lang = "reflector"]
 // If you're renaming or moving this field, update the path in plugins::reflector as well