From f6f0a7e4aaaa3c1ec7aca3876b0b0fe9e5fca9aa Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 21 Jul 2015 21:09:18 +0530 Subject: [PATCH 1/5] Make stmt part of unrooted_must_root handle type parameters (fixes #6651) --- components/plugins/lib.rs | 6 +- .../plugins/lints/unrooted_must_root.rs | 56 ++++++++++++++----- components/plugins/utils.rs | 14 ++++- components/script/dom/bindings/js.rs | 1 + components/script/dom/bindings/refcounted.rs | 1 + components/script/dom/bindings/trace.rs | 1 + components/script/dom/bindings/utils.rs | 3 +- 7 files changed, 64 insertions(+), 18 deletions(-) diff --git a/components/plugins/lib.rs b/components/plugins/lib.rs index 0a6c917fcf4..ba64164f9f6 100644 --- a/components/plugins/lib.rs +++ b/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); } diff --git a/components/plugins/lints/unrooted_must_root.rs b/components/plugins/lints/unrooted_must_root.rs index 38473305c17..be6175c2cce 100644 --- a/components/plugins/lints/unrooted_must_root.rs +++ b/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` 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); - 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)); - } + 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 {:?} 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 } - _ => {} - } + }) + } } diff --git a/components/plugins/utils.rs b/components/plugins/utils.rs index 8e4c8bcf495..9cd843ae5a7 100644 --- a/components/plugins/utils.rs +++ b/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)) +} diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index 36a3ce19198..a4f21a2a43d 100644 --- a/components/script/dom/bindings/js.rs +++ b/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 { /// Reference to rooted value that must not outlive this container ptr: NonZero<*const T>, diff --git a/components/script/dom/bindings/refcounted.rs b/components/script/dom/bindings/refcounted.rs index 4e8baee7c0b..e4017c684e8 100644 --- a/components/script/dom/bindings/refcounted.rs +++ b/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` instance. +#[allow_unrooted_interior] pub struct Trusted { /// A pointer to the Rust DOM object of type T, but void to allow /// sending `Trusted` between tasks, regardless of T's sendability. diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index f7f8c527d9a..ae3aefe01e6 100644 --- a/components/script/dom/bindings/trace.rs +++ b/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 { v: Vec } diff --git a/components/script/dom/bindings/utils.rs b/components/script/dom/bindings/utils.rs index 955dbfbf727..bde81857965 100644 --- a/components/script/dom/bindings/utils.rs +++ b/components/script/dom/bindings/utils.rs @@ -413,8 +413,7 @@ pub fn reflect_dom_object } /// 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, unused_attributes)] +#[allow(raw_pointer_derive, unrooted_must_root)] #[must_root] #[servo_lang = "reflector"] // If you're renaming or moving this field, update the path in plugins::reflector as well From 511e3337fb3b1839ac00039d56ef6861762ed5d1 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 21 Jul 2015 22:22:16 +0530 Subject: [PATCH 2/5] Fix rooting in script --- components/script/dom/bindings/js.rs | 6 ++++++ components/script/dom/node.rs | 29 ++++++++++++++-------------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index a4f21a2a43d..e5e9561a092 100644 --- a/components/script/dom/bindings/js.rs +++ b/components/script/dom/bindings/js.rs @@ -270,6 +270,12 @@ impl MutNullableHeap> { pub unsafe fn get_inner_as_layout(&self) -> Option> { self.ptr.get().map(|js| js.to_layout()) } + + /// Get a rooted value out of this object + // FIXME(#6684) + pub fn get_rooted(&self) -> Option> { + self.get().map(|o| o.root()) + } } impl Default for MutNullableHeap { diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index d7432c570b0..cdcc0b844af 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -328,14 +328,14 @@ impl<'a> PrivateNodeHelpers for &'a Node { match before { Some(ref before) => { assert!(before.parent_node.get().map(Root::from_rooted).r() == Some(self)); - match before.prev_sibling.get() { + let prev_sibling = before.prev_sibling.get_rooted(); + match prev_sibling { None => { assert!(Some(*before) == self.first_child.get().map(Root::from_rooted).r()); self.first_child.set(Some(JS::from_ref(new_child))); }, Some(ref prev_sibling) => { - let prev_sibling = prev_sibling.root(); - prev_sibling.r().next_sibling.set(Some(JS::from_ref(new_child))); + prev_sibling.next_sibling.set(Some(JS::from_ref(new_child))); new_child.prev_sibling.set(Some(JS::from_ref(prev_sibling.r()))); }, } @@ -343,11 +343,11 @@ impl<'a> PrivateNodeHelpers for &'a Node { new_child.next_sibling.set(Some(JS::from_ref(before))); }, None => { - match self.last_child.get() { + let last_child = self.last_child.get_rooted(); + match last_child { None => self.first_child.set(Some(JS::from_ref(new_child))), Some(ref last_child) => { - let last_child = last_child.root(); - assert!(last_child.r().next_sibling.get().is_none()); + assert!(last_child.next_sibling.get().is_none()); last_child.r().next_sibling.set(Some(JS::from_ref(new_child))); new_child.prev_sibling.set(Some(JS::from_rooted(&last_child))); } @@ -365,22 +365,22 @@ impl<'a> PrivateNodeHelpers for &'a Node { /// Fails unless `child` is a child of this node. fn remove_child(self, child: &Node) { assert!(child.parent_node.get().map(Root::from_rooted).r() == Some(self)); - - match child.prev_sibling.get() { + let prev_sibling = child.prev_sibling.get_rooted(); + match prev_sibling { None => { self.first_child.set(child.next_sibling.get()); } Some(ref prev_sibling) => { - prev_sibling.root().r().next_sibling.set(child.next_sibling.get()); + prev_sibling.next_sibling.set(child.next_sibling.get()); } } - - match child.next_sibling.get() { + let next_sibling = child.next_sibling.get_rooted(); + match next_sibling { None => { self.last_child.set(child.prev_sibling.get()); } Some(ref next_sibling) => { - next_sibling.root().r().prev_sibling.set(child.prev_sibling.get()); + next_sibling.prev_sibling.set(child.prev_sibling.get()); } } @@ -1476,9 +1476,10 @@ impl Node { // https://dom.spec.whatwg.org/#concept-node-adopt pub fn adopt(node: &Node, document: &Document) { // Step 1. - match node.parent_node.get() { + let parent_node = node.parent_node.get_rooted(); + match parent_node { Some(ref parent) => { - Node::remove(node, parent.root().r(), SuppressObserver::Unsuppressed); + Node::remove(node, parent, SuppressObserver::Unsuppressed); } None => (), } From fda3eb632703a3b11fe586516fe03c5e05a73a45 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 21 Jul 2015 23:24:42 +0530 Subject: [PATCH 3/5] Make struct part of unrooted_must_root handle type parameters --- .../plugins/lints/unrooted_must_root.rs | 64 +++++++++++-------- components/script/dom/browsercontext.rs | 1 + components/script/page.rs | 1 + components/script/script_task.rs | 2 + 4 files changed, 40 insertions(+), 28 deletions(-) diff --git a/components/plugins/lints/unrooted_must_root.rs b/components/plugins/lints/unrooted_must_root.rs index be6175c2cce..073212eabe6 100644 --- a/components/plugins/lints/unrooted_must_root.rs +++ b/components/plugins/lints/unrooted_must_root.rs @@ -57,6 +57,35 @@ fn lint_unrooted_ty(cx: &Context, ty: &ast::Ty, warning: &str) { }; } +fn is_unrooted_ty(cx: &Context, ty: &ty::TyS, in_new_function: bool) -> bool { + let mut ret = false; + ty.maybe_walk(|t| { + match t.sty { + ty::TyStruct(did, _) | + ty::TyEnum(did, _) => { + if cx.tcx.has_attr(did, "must_root") { + ret = true; + 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 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 + } + }); + ret +} + impl LintPass for UnrootedPass { fn get_lints(&self) -> LintArray { lint_array!(UNROOTED_MUST_ROOT) @@ -74,8 +103,9 @@ impl LintPass for UnrootedPass { }; if item.attrs.iter().all(|a| !a.check_name("must_root")) { for ref field in def.fields.iter() { - lint_unrooted_ty(cx, &*field.node.ty, - "Type must be rooted, use #[must_root] on the struct definition to propagate"); + if is_unrooted_ty(cx, cx.tcx.node_id_to_type(field.node.id), false) { + cx.span_lint(UNROOTED_MUST_ROOT, field.span, "Type must be rooted, use #[must_root] on the struct definition to propagate") + } } } } @@ -168,31 +198,9 @@ impl LintPass for UnrootedPass { }; 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 {:?} 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 - } - }) - + if is_unrooted_ty(cx, ty, self.in_new_function) { + cx.span_lint(UNROOTED_MUST_ROOT, expr.span, + &format!("Expression of type {:?} must be rooted", ty)) + } } } diff --git a/components/script/dom/browsercontext.rs b/components/script/dom/browsercontext.rs index 595153f1fc3..cc4dc346c3f 100644 --- a/components/script/dom/browsercontext.rs +++ b/components/script/dom/browsercontext.rs @@ -30,6 +30,7 @@ use std::default::Default; #[derive(JSTraceable)] #[privatize] #[allow(raw_pointer_derive)] +#[must_root] pub struct BrowsingContext { history: Vec, active_index: usize, diff --git a/components/script/page.rs b/components/script/page.rs index 563cbae1006..3d9dc1d8da7 100644 --- a/components/script/page.rs +++ b/components/script/page.rs @@ -14,6 +14,7 @@ use std::rc::Rc; /// Encapsulates a handle to a frame in a frame tree. #[derive(JSTraceable)] +#[allow(unrooted_must_root)] // FIXME(#6686) this is wrong pub struct Page { /// Pipeline id associated with this page. id: PipelineId, diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 5b91fe8c5ac..51937350df2 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -287,6 +287,8 @@ impl Drop for StackRootTLS { /// Information for an entire page. Pages are top-level browsing contexts and can contain multiple /// frames. #[derive(JSTraceable)] +// ScriptTask instances are rooted on creation, so this is okay +#[allow(unrooted_must_root)] pub struct ScriptTask { /// A handle to the information pertaining to page layout page: DOMRefCell>>, From 521d8bc32e25ee31a0ccc9de77720207b69ac358 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Tue, 21 Jul 2015 23:41:19 +0530 Subject: [PATCH 4/5] Make enum/fn part of unrooted_must_root handle type parameters --- .../plugins/lints/unrooted_must_root.rs | 43 ++++++++----------- components/script/dom/bindings/js.rs | 1 + components/script/page.rs | 2 +- 3 files changed, 19 insertions(+), 27 deletions(-) diff --git a/components/plugins/lints/unrooted_must_root.rs b/components/plugins/lints/unrooted_must_root.rs index 073212eabe6..55607b39b30 100644 --- a/components/plugins/lints/unrooted_must_root.rs +++ b/components/plugins/lints/unrooted_must_root.rs @@ -6,7 +6,8 @@ use syntax::{ast, codemap, visit}; use syntax::attr::AttrMetaMethods; use rustc::ast_map; use rustc::lint::{Context, LintPass, LintArray}; -use rustc::middle::{ty, def}; +use rustc::middle::ty; +use rustc::middle::astconv_util::ast_ty_to_prim_ty; use utils::{match_def_path, unsafe_context}; declare_lint!(UNROOTED_MUST_ROOT, Deny, @@ -36,27 +37,8 @@ impl UnrootedPass { } } } -// 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 -fn lint_unrooted_ty(cx: &Context, ty: &ast::Ty, warning: &str) { - match ty.node { - ast::TyVec(ref t) | ast::TyFixedLengthVec(ref t, _) => - lint_unrooted_ty(cx, &**t, warning), - ast::TyPath(..) => { - match cx.tcx.def_map.borrow()[&ty.id] { - def::PathResolution{ base_def: def::DefTy(def_id, _), .. } => { - if cx.tcx.has_attr(def_id, "must_root") { - cx.span_lint(UNROOTED_MUST_ROOT, ty.span, warning); - } - } - _ => (), - } - } - _ => (), - }; -} +/// Checks if a type is unrooted or contains any owned unrooted types fn is_unrooted_ty(cx: &Context, ty: &ty::TyS, in_new_function: bool) -> bool { let mut ret = false; ty.maybe_walk(|t| { @@ -104,7 +86,8 @@ impl LintPass for UnrootedPass { if item.attrs.iter().all(|a| !a.check_name("must_root")) { for ref field in def.fields.iter() { if is_unrooted_ty(cx, cx.tcx.node_id_to_type(field.node.id), false) { - cx.span_lint(UNROOTED_MUST_ROOT, field.span, "Type must be rooted, use #[must_root] on the struct definition to propagate") + cx.span_lint(UNROOTED_MUST_ROOT, field.span, + "Type must be rooted, use #[must_root] on the struct definition to propagate") } } } @@ -116,8 +99,13 @@ impl LintPass for UnrootedPass { match var.node.kind { ast::TupleVariantKind(ref vec) => { for ty in vec.iter() { - lint_unrooted_ty(cx, &*ty.ty, - "Type must be rooted, use #[must_root] on the enum definition to propagate") + ast_ty_to_prim_ty(cx.tcx, &*ty.ty).map(|t| { + if is_unrooted_ty(cx, t, false) { + cx.span_lint(UNROOTED_MUST_ROOT, ty.ty.span, + "Type must be rooted, use #[must_root] on \ + the enum definition to propagate") + } + }); } } _ => () // Struct variants already caught by check_struct_def @@ -150,8 +138,11 @@ impl LintPass for UnrootedPass { match block.rules { ast::DefaultBlock => { for arg in decl.inputs.iter() { - lint_unrooted_ty(cx, &*arg.ty, - "Type must be rooted") + ast_ty_to_prim_ty(cx.tcx, &*arg.ty).map(|t| { + if is_unrooted_ty(cx, t, false) { + cx.span_lint(UNROOTED_MUST_ROOT, arg.ty.span, "Type must be rooted") + } + }); } } _ => () // fn is `unsafe` diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index e5e9561a092..fc929db1cac 100644 --- a/components/script/dom/bindings/js.rs +++ b/components/script/dom/bindings/js.rs @@ -81,6 +81,7 @@ impl JS { /// An unrooted reference to a DOM object for use in layout. `Layout*Helpers` /// traits must be implemented on this. +#[allow_unrooted_interior] pub struct LayoutJS { ptr: NonZero<*const T> } diff --git a/components/script/page.rs b/components/script/page.rs index 3d9dc1d8da7..1286a1427f1 100644 --- a/components/script/page.rs +++ b/components/script/page.rs @@ -14,7 +14,7 @@ use std::rc::Rc; /// Encapsulates a handle to a frame in a frame tree. #[derive(JSTraceable)] -#[allow(unrooted_must_root)] // FIXME(#6686) this is wrong +#[allow(unrooted_must_root)] // FIXME(#6687) this is wrong pub struct Page { /// Pipeline id associated with this page. id: PipelineId, From a9f651cfa1db2fad8a7851a5bf2bca4dc793a3a8 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 22 Jul 2015 18:28:02 +0530 Subject: [PATCH 5/5] Address review comments; add docs --- components/plugins/lints/unrooted_must_root.rs | 4 ++++ components/script/dom/node.rs | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/components/plugins/lints/unrooted_must_root.rs b/components/plugins/lints/unrooted_must_root.rs index 55607b39b30..d51fe19871e 100644 --- a/components/plugins/lints/unrooted_must_root.rs +++ b/components/plugins/lints/unrooted_must_root.rs @@ -26,6 +26,10 @@ declare_lint!(UNROOTED_MUST_ROOT, Deny, /// /// This helps catch most situations where pointers like `JS` are used in a way that they can be invalidated by a /// GC pass. +/// +/// Structs which have their own mechanism of rooting their unrooted contents (e.g. `ScriptTask`) +/// can be marked as `#[allow(unrooted_must_root)]`. Smart pointers which root their interior type +/// can be marked as `#[allow_unrooted_interior]` pub struct UnrootedPass { in_new_function: bool } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index cdcc0b844af..e6290861c73 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -328,7 +328,7 @@ impl<'a> PrivateNodeHelpers for &'a Node { match before { Some(ref before) => { assert!(before.parent_node.get().map(Root::from_rooted).r() == Some(self)); - let prev_sibling = before.prev_sibling.get_rooted(); + let prev_sibling = before.GetPreviousSibling(); match prev_sibling { None => { assert!(Some(*before) == self.first_child.get().map(Root::from_rooted).r()); @@ -343,7 +343,7 @@ impl<'a> PrivateNodeHelpers for &'a Node { new_child.next_sibling.set(Some(JS::from_ref(before))); }, None => { - let last_child = self.last_child.get_rooted(); + let last_child = self.GetLastChild(); match last_child { None => self.first_child.set(Some(JS::from_ref(new_child))), Some(ref last_child) => { @@ -365,7 +365,7 @@ impl<'a> PrivateNodeHelpers for &'a Node { /// Fails unless `child` is a child of this node. fn remove_child(self, child: &Node) { assert!(child.parent_node.get().map(Root::from_rooted).r() == Some(self)); - let prev_sibling = child.prev_sibling.get_rooted(); + let prev_sibling = child.GetPreviousSibling(); match prev_sibling { None => { self.first_child.set(child.next_sibling.get()); @@ -374,7 +374,7 @@ impl<'a> PrivateNodeHelpers for &'a Node { prev_sibling.next_sibling.set(child.next_sibling.get()); } } - let next_sibling = child.next_sibling.get_rooted(); + let next_sibling = child.GetNextSibling(); match next_sibling { None => { self.last_child.set(child.prev_sibling.get()); @@ -1476,7 +1476,7 @@ impl Node { // https://dom.spec.whatwg.org/#concept-node-adopt pub fn adopt(node: &Node, document: &Document) { // Step 1. - let parent_node = node.parent_node.get_rooted(); + let parent_node = node.GetParentNode(); match parent_node { Some(ref parent) => { Node::remove(node, parent, SuppressObserver::Unsuppressed);