servo: Merge #3903 - Improve docs and #[must_root] checking in plugins crate (from Manishearth:plugin-fixes); r=jdm

Source-Repo: https://github.com/servo/servo
Source-Revision: 57cb8a10f0f6ed445ed7ba88a3fa19d56bd7015e

servo/components/plugins/jstraceable.rs

@@ -20,6 +20,9 @@ pub fn expand_dom_struct(_: &mut ExtCtxt, _: Span, _: &MetaItem, item: P<Item>)
     P(item2)
 }
 
+/// Provides the hook to expand `#[jstraceable]` into an implementation of `JSTraceable`
+///
+/// The expansion basically calls `trace()` on all of the fields of the struct/enum, erroring if they do not implement the method.
 pub fn expand_jstraceable(cx: &mut ExtCtxt, span: Span, mitem: &MetaItem, item: &Item, push: |P<Item>|) {
     let trait_def = TraitDef {
         span: span,
@@ -45,6 +48,7 @@ pub fn expand_jstraceable(cx: &mut ExtCtxt, span: Span, mitem: &MetaItem, item:
 }
 
 // Mostly copied from syntax::ext::deriving::hash
+/// Defines how the implementation for `trace()` is to be generated
 fn jstraceable_substructure(cx: &mut ExtCtxt, trait_span: Span, substr: &Substructure) -> P<Expr> {
     let state_expr = match substr.nonself_args {
         [ref state_expr] => state_expr,

servo/components/plugins/lib.rs

@@ -2,6 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+//! Servo's compiler plugin/macro crate
+//!
+//! Attributes this crate provides:
+//!
+//!  - `#[privatize]` : Forces all fields in a struct/enum to be private
+//!  - `#[jstraceable]` : Auto-derives an implementation of `JSTraceable` for a struct in the script crate
+//!  - `#[must_root]` : Prevents data of the marked type from being used on the stack. See the lints module for more details
+//!  - `#[dom_struct]` : Implies `#[privatize]`,`#[jstraceable]`, and `#[must_root]`.
+//!     Use this for structs that correspond to a DOM type
+
 #![feature(macro_rules, plugin_registrar, quote, phase)]
 
 #![deny(unused_imports, unused_variable)]
@@ -19,10 +29,12 @@ use syntax::ext::base::{Decorator, Modifier};
 
 use syntax::parse::token::intern;
 
-mod lints;
+// Public for documentation to show up
-mod macros;
+/// Handles the auto-deriving for `#[jstraceable]`
-mod jstraceable;
+pub mod jstraceable;
+pub mod lints;
 
+mod macros;
 #[plugin_registrar]
 pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_syntax_extension(intern("dom_struct"), Modifier(box jstraceable::expand_dom_struct));

servo/components/plugins/lints.rs

@@ -18,8 +18,27 @@ declare_lint!(UNROOTED_MUST_ROOT, Deny,
 declare_lint!(PRIVATIZE, Deny,
               "Allows to enforce private fields for struct definitions")
 
+/// Lint for auditing transmutes
+///
+/// This lint (off by default, enable with `-W transmute-type-lint`) warns about all the transmutes
+/// being used, along with the types they transmute to/from.
 pub struct TransmutePass;
+
+/// Lint for ensuring safe usage of unrooted pointers
+///
+/// This lint (disable with `-A unrooted-must-root`/`#[allow(unrooted_must_root)]`) ensures that `#[must_root]` values are used correctly.
+/// "Incorrect" usage includes:
+///
+///  - Not being used in a struct/enum field which is not `#[must_root]` itself
+///  - Not being used as an argument to a function (Except onces named `new` and `new_inherited`)
+///  - Not being bound locally in a `let` statement, assignment, `for` loop, or `match` statement.
+///
+/// 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;
+
+/// Lint for keeping DOM fields private
+///
+/// This lint (disable with `-A privatize`/`#[allow(privatize)]`) ensures all types marked with `#[privatize]` have no private fields
 pub struct PrivatizePass;
 
 impl LintPass for TransmutePass {
@@ -51,6 +70,9 @@ impl LintPass for TransmutePass {
     }
 }
 
+// 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::TyBox(ref t) | ast::TyUniq(ref t) |
@@ -74,7 +96,7 @@ impl LintPass for UnrootedPass {
     fn get_lints(&self) -> LintArray {
         lint_array!(UNROOTED_MUST_ROOT)
     }
-
+    /// All structs containing #[must_root] types must be #[must_root] themselves
     fn check_struct_def(&mut self, cx: &Context, def: &ast::StructDef, _i: ast::Ident, _gen: &ast::Generics, id: ast::NodeId) {
         if cx.tcx.map.expect_item(id).attrs.iter().all(|a| !a.check_name("must_root")) {
             for ref field in def.fields.iter() {
@@ -83,7 +105,7 @@ impl LintPass for UnrootedPass {
             }
         }
     }
-
+    /// All enums containing #[must_root] types must be #[must_root] themselves
     fn check_variant(&mut self, cx: &Context, var: &ast::Variant, _gen: &ast::Generics) {
         let ref map = cx.tcx.map;
         if map.expect_item(map.get_parent(var.node.id)).attrs.iter().all(|a| !a.check_name("must_root")) {
@@ -98,7 +120,7 @@ impl LintPass for UnrootedPass {
             }
         }
     }
-
+    /// Function arguments that are #[must_root] types are not allowed
     fn check_fn(&mut self, cx: &Context, kind: visit::FnKind, decl: &ast::FnDecl,
                 block: &ast::Block, _span: codemap::Span, _id: ast::NodeId) {
         match kind {
@@ -120,19 +142,32 @@ impl LintPass for UnrootedPass {
     }
 
     // Partially copied from rustc::middle::lint::builtin
-    // Catches `let` statements which store a #[must_root] value
+    // Catches `let` statements and assignments which store a #[must_root] value
-    // Expressions which return out of blocks eventually end up in a `let`
+    // 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) {
-        // Catch the let binding
         let expr = match s.node {
+            // Catch a `let` binding
             ast::StmtDecl(ref decl, _) => match decl.node {
                 ast::DeclLocal(ref loc) => match loc.init {
-                        Some(ref e) => &**e,
+                    Some(ref e) => &**e,
-                        _ => return
+                    _ => return
                 },
                 _ => return
             },
+            ast::StmtExpr(ref expr, _) => match expr.node {
+                // This catches deferred `let` statements
+                ast::ExprAssign(_, ref e) |
+                // Match statements allow you to bind onto the variable later in an arm
+                // We need not check arms individually since enum/struct fields are already
+                // linted in `check_struct_def` and `check_variant`
+                // (so there is no way of destructuring out a `#[must_root]` field)
+                ast::ExprMatch(ref e, _) |
+                // For loops allow you to bind a return value locally
+                ast::ExprForLoop(_, ref e, _, _) => &**e,
+                // XXXManishearth look into `if let` once it lands in our rustc
+                _ => return
+            },
             _ => return
         };
 

servo/components/plugins/macros.rs

@@ -2,8 +2,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-//! Exports macros for use in other Servo crates.
-
 #[macro_export]
 macro_rules! bitfield(
     ($bitfieldname:ident, $getter:ident, $setter:ident, $value:expr) => (

servo/components/script/dom/xmlhttprequest.rs

@@ -13,7 +13,7 @@ use dom::bindings::codegen::InheritTypes::{EventCast, EventTargetCast, XMLHttpRe
 use dom::bindings::conversions::ToJSValConvertible;
 use dom::bindings::error::{Error, ErrorResult, Fallible, InvalidState, InvalidAccess};
 use dom::bindings::error::{Network, Syntax, Security, Abort, Timeout};
-use dom::bindings::global::{GlobalField, GlobalRef, WorkerField};
+use dom::bindings::global::{GlobalField, GlobalRef, WorkerRoot};
 use dom::bindings::js::{MutNullableJS, JS, JSRef, Temporary, OptionalRootedRootable};
 use dom::bindings::str::ByteString;
 use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
@@ -698,8 +698,8 @@ impl<'a> XMLHttpRequestMethods for JSRef<'a, XMLHttpRequest> {
         self.response_type.get()
     }
     fn SetResponseType(self, response_type: XMLHttpRequestResponseType) -> ErrorResult {
-        match self.global {
+        match self.global.root() {
-            WorkerField(_) if response_type == XMLHttpRequestResponseTypeValues::Document
+            WorkerRoot(_) if response_type == XMLHttpRequestResponseTypeValues::Document
             => return Ok(()),
             _ => {}
         }