Auto merge of #5855 - Manishearth:nomove, r=kmc,munksgaard

fixes #5724, #5737


uses https://github.com/Manishearth/rust-tenacious (can be moved in-tree if needed)

I can make it `Deny` by default too (I'll add a cargo feature to tenacious), though we might want it on
`Warn` until we get some mileage on it.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5855)
<!-- Reviewable:end -->

components/plugins/Cargo.toml

@@ -7,3 +7,6 @@ authors = ["The Servo Project Developers"]
 name = "plugins"
 path = "lib.rs"
 plugin = true
+
+[dependencies.tenacious]
+git = "https://github.com/Manishearth/rust-tenacious.git"

components/plugins/lib.rs

@@ -19,6 +19,8 @@ extern crate syntax;
 #[macro_use]
 extern crate rustc;
 
+extern crate tenacious;
+
 use rustc::lint::LintPassObject;
 use rustc::plugin::Registry;
 use syntax::ext::base::{Decorator, Modifier};
@@ -48,4 +50,5 @@ pub fn plugin_registrar(reg: &mut Registry) {
     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);
 }

components/script/dom/bindings/codegen/CodegenRust.py

@@ -4492,6 +4492,7 @@ class CGDictionary(CGThing):
                        for m in self.memberInfo]
 
         return (string.Template(
+                "#[no_move]\n" +
                 "pub struct ${selfName} {\n" +
                 "${inheritance}" +
                 "\n".join(memberDecls) + "\n" +

components/script/dom/bindings/global.rs

@@ -34,6 +34,7 @@ pub enum GlobalRef<'a> {
 }
 
 /// A stack-based rooted reference to a global object.
+#[no_move]
 pub enum GlobalRoot {
     /// A root for a `Window` object.
     Window(Root<window::Window>),

components/script/dom/bindings/js.rs

@@ -577,6 +577,7 @@ impl<T: Assignable<U>, U: Reflectable> TemporaryPushable<T> for Vec<JS<U>> {
 ///
 /// See also [*Exact Stack Rooting - Storing a GCPointer on the CStack*]
 /// (https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Internals/GC/Exact_Stack_Rooting).
+#[no_move]
 pub struct RootCollection {
     roots: UnsafeCell<RootedVec<*mut JSObject>>,
 }
@@ -626,6 +627,7 @@ impl RootCollection {
 /// for the same JS value. `Root`s cannot outlive the associated
 /// `RootCollection` object. Attempts to transfer ownership of a `Root` via
 /// moving will trigger dynamic unrooting failures due to incorrect ordering.
+#[no_move]
 pub struct Root<T> {
     /// List that ensures correct dynamic root ordering
     root_list: &'static RootCollection,

components/script/dom/bindings/mod.rs

@@ -152,7 +152,9 @@ pub mod trace;
 /// Generated JS-Rust bindings.
 #[allow(missing_docs, non_snake_case)]
 pub mod codegen {
-    #[allow(unrooted_must_root)]
+    // FIXME(#5853) we shouldn't need to
+    // allow moved_no_move here
+    #[allow(unrooted_must_root, moved_no_move)]
     pub mod Bindings;
     pub mod InterfaceTypes;
     pub mod InheritTypes;

components/script/dom/bindings/trace.rs

@@ -401,6 +401,7 @@ impl VecRootableType for *mut JSObject {
 /// A vector of items that are rooted for the lifetime
 /// of this struct
 #[allow(unrooted_must_root)]
+#[no_move]
 pub struct RootedVec<T> {
     v: Vec<T>
 }

components/script/dom/document.rs

@@ -1195,7 +1195,7 @@ impl<'a> DocumentMethods for JSRef<'a, Document> {
 
         match title {
             None => DOMString::new(),
-            Some(title) => {
+            Some(ref title) => {
                 // Steps 3-4.
                 let value = Node::collect_text_contents(title.r().children());
                 split_html_space_chars(&value).collect::<Vec<_>>().connect(" ")
@@ -1310,9 +1310,9 @@ impl<'a> DocumentMethods for JSRef<'a, Document> {
             return Ok(());
         }
 
-        match (self.get_html_element().root(), old_body) {
+        match (self.get_html_element().root(), &old_body) {
             // Step 3.
-            (Some(ref root), Some(ref child)) => {
+            (Some(ref root), &Some(ref child)) => {
                 let root: JSRef<Node> = NodeCast::from_ref(root.r());
                 let child: JSRef<Node> = NodeCast::from_ref(child.r());
                 let new_body: JSRef<Node> = NodeCast::from_ref(new_body);
@@ -1323,7 +1323,7 @@ impl<'a> DocumentMethods for JSRef<'a, Document> {
             (None, _) => return Err(HierarchyRequest),
 
             // Step 5.
-            (Some(ref root), None) => {
+            (Some(ref root), &None) => {
                 let root: JSRef<Node> = NodeCast::from_ref(root.r());
                 let new_body: JSRef<Node> = NodeCast::from_ref(new_body);
                 assert!(root.AppendChild(new_body).is_ok());

components/script/dom/domimplementation.rs

@@ -92,7 +92,7 @@ impl<'a> DOMImplementationMethods for JSRef<'a, DOMImplementation> {
             // Step 5.
             match maybe_elem.root() {
                 None => (),
-                Some(elem) => {
+                Some(ref elem) => {
                     assert!(doc_node.AppendChild(NodeCast::from_ref(elem.r())).is_ok())
                 }
             }

components/script/dom/element.rs

@@ -728,7 +728,7 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> {
     fn get_attributes(self, local_name: &Atom, attributes: &mut RootedVec<JS<Attr>>) {
         // FIXME(https://github.com/rust-lang/rust/issues/23338)
         let attrs = self.attrs.borrow();
-        for attr in attrs.iter().map(|attr| attr.root()) {
+        for ref attr in attrs.iter().map(|attr| attr.root()) {
             // FIXME(https://github.com/rust-lang/rust/issues/23338)
             let attr = attr.r();
             let attr_local_name = attr.local_name();
@@ -964,7 +964,7 @@ impl<'a> AttributeHandlers for JSRef<'a, Element> {
         }));
         let attribute = self.get_attribute(&ns!(""), local_name).root();
         match attribute {
-            Some(attribute) => {
+            Some(ref attribute) => {
                 match *attribute.r().value() {
                     AttrValue::UInt(_, value) => value,
                     _ => panic!("Expected an AttrValue::UInt: \
@@ -1457,7 +1457,7 @@ impl<'a> VirtualMethods for JSRef<'a, Element> {
 
         if !tree_in_doc { return; }
 
-        if let Some(attr) = self.get_attribute(&ns!(""), &atom!("id")).root() {
+        if let Some(ref attr) = self.get_attribute(&ns!(""), &atom!("id")).root() {
             let doc = document_from_node(*self).root();
             let value = attr.r().Value();
             if !value.is_empty() {
@@ -1474,7 +1474,7 @@ impl<'a> VirtualMethods for JSRef<'a, Element> {
 
         if !tree_in_doc { return; }
 
-        if let Some(attr) = self.get_attribute(&ns!(""), &atom!("id")).root() {
+        if let Some(ref attr) = self.get_attribute(&ns!(""), &atom!("id")).root() {
             let doc = document_from_node(*self).root();
             let value = attr.r().Value();
             if !value.is_empty() {
@@ -1698,7 +1698,7 @@ impl<'a> ActivationElementHelpers<'a> for JSRef<'a, Element> {
         // Step 4
         let e = self.nearest_activable_element().root();
         match e {
-            Some(el) => match el.r().as_maybe_activatable() {
+            Some(ref el) => match el.r().as_maybe_activatable() {
                 Some(elem) => {
                     // Step 5-6
                     elem.pre_click_activation();

components/script/dom/eventdispatcher.rs

@@ -115,7 +115,7 @@ pub fn dispatch_event<'a, 'b>(target: JSRef<'a, EventTarget>,
     /* default action */
     let target = event.GetTarget().root();
     match target {
-        Some(target) => {
+        Some(ref target) => {
             let node: Option<JSRef<Node>> = NodeCast::to_ref(target.r());
             match node {
                 Some(node) => {

components/script/dom/htmlformelement.rs

@@ -522,7 +522,7 @@ pub trait FormControl<'a> : Copy + Sized {
             let doc = document_from_node(elem).root();
             let owner = doc.r().GetElementById(owner).root();
             match owner {
-                Some(o) => {
+                Some(ref o) => {
                     let maybe_form: Option<JSRef<HTMLFormElement>> = HTMLFormElementCast::to_ref(o.r());
                     if maybe_form.is_some() {
                         return maybe_form.map(Temporary::from_rooted);

components/script/dom/htmlinputelement.rs

@@ -354,7 +354,7 @@ fn broadcast_radio_checked(broadcaster: JSRef<HTMLInputElement>, group: Option<&
                 .map(|t| t.root())
                 .filter(|r| in_same_group(r.r(), owner, group) && broadcaster != r.r())
         };
-        for r in iter {
+        for ref r in iter {
             if r.r().Checked() {
                 r.r().SetChecked(false);
             }
@@ -743,7 +743,7 @@ impl<'a> Activatable for JSRef<'a, HTMLInputElement> {
                     let old_checked: Option<Root<HTMLInputElement>> = cache.checked_radio.get().root();
                     let name = self.get_radio_group_name();
                     match old_checked {
-                        Some(o) => {
+                        Some(ref o) => {
                             // Avoiding iterating through the whole tree here, instead
                             // we can check if the conditions for radio group siblings apply
                             if name == o.r().get_radio_group_name() && // TODO should be compatibility caseless
@@ -841,7 +841,7 @@ impl<'a> Activatable for JSRef<'a, HTMLInputElement> {
                 .find(|r| r.r().form_owner() == owner);
         }
         match submit_button {
-            Some(button) => {
+            Some(ref button) => {
                 if button.r().is_instance_activatable() {
                     button.r().synthetic_click_activation(ctrlKey, shiftKey, altKey, metaKey)
                 }

components/script/dom/htmllinkelement.rs

@@ -9,7 +9,7 @@ use dom::bindings::codegen::Bindings::HTMLLinkElementBinding::HTMLLinkElementMet
 use dom::bindings::codegen::InheritTypes::HTMLLinkElementDerived;
 use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLElementCast, NodeCast};
 use dom::bindings::js::{JS, JSRef, MutNullableHeap, Temporary};
-use dom::bindings::js::OptionalRootable;
+use dom::bindings::js::{OptionalRootable, RootedReference};
 use dom::document::Document;
 use dom::domtokenlist::DOMTokenList;
 use dom::element::{AttributeHandlers, Element};
@@ -60,9 +60,7 @@ impl HTMLLinkElement {
 
 fn get_attr(element: JSRef<Element>, local_name: &Atom) -> Option<String> {
     let elem = element.get_attribute(&ns!(""), local_name).root();
-    elem.map(|e| {
+    elem.r().map(|e| {
-        // FIXME(https://github.com/rust-lang/rust/issues/23338)
-        let e = e.r();
         let value = e.value();
         (**value).to_owned()
     })

components/script/dom/htmlscriptelement.rs

@@ -223,7 +223,7 @@ impl<'a> HTMLScriptElementHelpers for JSRef<'a, HTMLScriptElement> {
         }
 
         // Step 13.
-        if let Some(charset) = element.get_attribute(&ns!(""), &Atom::from_slice("charset")).root() {
+        if let Some(ref charset) = element.get_attribute(&ns!(""), &Atom::from_slice("charset")).root() {
             if let Some(encodingRef) = encoding_from_whatwg_label(&charset.r().Value()) {
                 *self.block_character_encoding.borrow_mut() = encodingRef;
             }
@@ -236,7 +236,7 @@ impl<'a> HTMLScriptElementHelpers for JSRef<'a, HTMLScriptElement> {
 
         let load = match element.get_attribute(&ns!(""), &atom!("src")).root() {
             // Step 14.
-            Some(src) => {
+            Some(ref src) => {
                 // Step 14.1
                 let src = src.r().Value();
 

components/script/dom/node.rs

@@ -298,7 +298,7 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
         }
 
         let parent = self.parent_node().root();
-        parent.map(|parent| vtable_for(&parent.r()).child_inserted(self));
+        parent.r().map(|parent| vtable_for(&parent).child_inserted(self));
         document.r().content_and_heritage_changed(self, NodeDamage::OtherNodeDamage);
     }
 
@@ -331,7 +331,7 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
                         assert!(Some(*before) == self.first_child().root().r());
                         self.first_child.set(Some(JS::from_rooted(new_child)));
                     },
-                    Some(prev_sibling) => {
+                    Some(ref prev_sibling) => {
                         prev_sibling.r().next_sibling.set(Some(JS::from_rooted(new_child)));
                         new_child.prev_sibling.set(Some(JS::from_rooted(prev_sibling.r())));
                     },
@@ -342,7 +342,7 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
             None => {
                 match self.last_child().root() {
                     None => self.first_child.set(Some(JS::from_rooted(new_child))),
-                    Some(last_child) => {
+                    Some(ref last_child) => {
                         assert!(last_child.r().next_sibling().is_none());
                         last_child.r().next_sibling.set(Some(JS::from_rooted(new_child)));
                         new_child.prev_sibling.set(Some(JS::from_rooted(last_child.r())));
@@ -366,7 +366,7 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
             None => {
                 self.first_child.set(child.next_sibling.get());
             }
-            Some(prev_sibling) => {
+            Some(ref prev_sibling) => {
                 prev_sibling.r().next_sibling.set(child.next_sibling.get());
             }
         }
@@ -375,7 +375,7 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
             None => {
                 self.last_child.set(child.prev_sibling.get());
             }
-            Some(next_sibling) => {
+            Some(ref next_sibling) => {
                 next_sibling.r().prev_sibling.set(child.prev_sibling.get());
             }
         }
@@ -978,7 +978,7 @@ impl<'a> NodeHelpers for JSRef<'a, Node> {
 
     fn remove_self(self) {
         match self.parent_node().root() {
-            Some(parent) => parent.r().remove_child(self),
+            Some(ref parent) => parent.r().remove_child(self),
             None => ()
         }
     }
@@ -1377,7 +1377,7 @@ impl Node {
     pub fn adopt(node: JSRef<Node>, document: JSRef<Document>) {
         // Step 1.
         match node.parent_node().root() {
-            Some(parent) => {
+            Some(ref parent) => {
                 Node::remove(node, parent.r(), SuppressObserver::Unsuppressed);
             }
             None => (),
@@ -1773,7 +1773,7 @@ impl Node {
 
                 // FIXME: https://github.com/mozilla/servo/issues/1737
                 let window = document.window().root();
-                for attr in node_elem.attrs().iter().map(|attr| attr.root()) {
+                for ref attr in node_elem.attrs().iter().map(|attr| attr.root()) {
                     copy_elem.attrs_mut().push_unrooted(
                         &Attr::new(window.r(),
                                    attr.r().local_name().clone(), attr.r().value().clone(),
@@ -2581,7 +2581,7 @@ impl<'a> DisabledStateHelpers for JSRef<'a, Node> {
                           .map(|child| child.root())
                           .find(|child| child.r().is_htmllegendelement())
             {
-                Some(legend) => {
+                Some(ref legend) => {
                     // XXXabinader: should we save previous ancestor to avoid this iteration?
                     if self.ancestors().any(|ancestor| ancestor.root().r() == legend.r()) { continue; }
                 },

components/script/dom/text.rs

@@ -94,7 +94,7 @@ impl<'a> TextMethods for JSRef<'a, Text> {
         let nodes = first.r().inclusively_following_siblings().map(|node| node.root())
                              .take_while(|node| node.r().is_text());
         let mut text = DOMString::new();
-        for node in nodes {
+        for ref node in nodes {
             let cdata = CharacterDataCast::to_ref(node.r()).unwrap();
             text.push_str(&cdata.data());
         }

components/script/script_task.rs

@@ -900,7 +900,7 @@ impl ScriptTask {
         let doc = page.document().root();
         let frame_element = self.find_iframe(doc.r(), subpage_id).root();
 
-        if let Some(frame_element) = frame_element {
+        if let Some(ref frame_element) = frame_element {
             let element: JSRef<Element> = ElementCast::from_ref(frame_element.r());
             doc.r().begin_focus_transaction();
             doc.r().request_focus(element);
@@ -921,7 +921,7 @@ impl ScriptTask {
             self.find_iframe(doc.r(), subpage_id)
         }).root();
 
-        if let Some(frame_element) = frame_element {
+        if let Some(ref frame_element) = frame_element {
             frame_element.r().dispatch_mozbrowser_event(event);
         }
     }
@@ -937,7 +937,7 @@ impl ScriptTask {
             self.find_iframe(doc.r(), old_subpage_id)
         }).root();
 
-        frame_element.unwrap().r().update_subpage_id(new_subpage_id);
+        frame_element.r().unwrap().update_subpage_id(new_subpage_id);
     }
 
     /// Handles a notification that reflow completed.
@@ -1295,7 +1295,7 @@ impl ScriptTask {
         let page = get_page(&self.root_page(), pipeline_id);
         let document = page.document().root();
         match document.r().find_fragment_node(fragment).root() {
-            Some(node) => {
+            Some(ref node) => {
                 self.scroll_fragment_point(pipeline_id, node.r());
             }
             None => {}
@@ -1314,7 +1314,7 @@ impl ScriptTask {
                                       .and_then(|name| document.r().find_fragment_node(name))
                                       .root();
         match fragment_node {
-            Some(node) => self.scroll_fragment_point(pipeline_id, node.r()),
+            Some(ref node) => self.scroll_fragment_point(pipeline_id, node.r()),
             None => {}
         }
 

components/servo/Cargo.lock

@@ -781,6 +781,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 [[package]]
 name = "plugins"
 version = "0.0.1"
+dependencies = [
+ "tenacious 0.0.1 (git+https://github.com/Manishearth/rust-tenacious.git)",
+]
 
 [[package]]
 name = "pnacl-build-helper"
@@ -980,6 +983,11 @@ dependencies = [
 name = "task_info"
 version = "0.0.1"
 
+[[package]]
+name = "tenacious"
+version = "0.0.1"
+source = "git+https://github.com/Manishearth/rust-tenacious.git#d61782e70005a9f0cdf66f366d4ec88fc563ea1e"
+
 [[package]]
 name = "text_writer"
 version = "0.1.9"

ports/cef/Cargo.lock

@@ -759,6 +759,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 [[package]]
 name = "plugins"
 version = "0.0.1"
+dependencies = [
+ "tenacious 0.0.1 (git+https://github.com/Manishearth/rust-tenacious.git)",
+]
 
 [[package]]
 name = "pnacl-build-helper"
@@ -960,6 +963,11 @@ dependencies = [
 name = "task_info"
 version = "0.0.1"
 
+[[package]]
+name = "tenacious"
+version = "0.0.1"
+source = "git+https://github.com/Manishearth/rust-tenacious.git#d61782e70005a9f0cdf66f366d4ec88fc563ea1e"
+
 [[package]]
 name = "text_writer"
 version = "0.1.9"

ports/gonk/Cargo.lock

@@ -751,6 +751,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 [[package]]
 name = "plugins"
 version = "0.0.1"
+dependencies = [
+ "tenacious 0.0.1 (git+https://github.com/Manishearth/rust-tenacious.git)",
+]
 
 [[package]]
 name = "pnacl-build-helper"
@@ -951,6 +954,11 @@ dependencies = [
 name = "task_info"
 version = "0.0.1"
 
+[[package]]
+name = "tenacious"
+version = "0.0.1"
+source = "git+https://github.com/Manishearth/rust-tenacious.git#d61782e70005a9f0cdf66f366d4ec88fc563ea1e"
+
 [[package]]
 name = "text_writer"
 version = "0.1.9"