From b52e311bece685e2380097035c9bfe1e95cc4848 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 5 Nov 2014 20:03:34 -0700 Subject: [PATCH] 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 | 4 ++ servo/components/plugins/lib.rs | 18 +++++-- servo/components/plugins/lints.rs | 51 ++++++++++++++++--- servo/components/plugins/macros.rs | 2 - servo/components/script/dom/xmlhttprequest.rs | 6 +-- 5 files changed, 65 insertions(+), 16 deletions(-) diff --git a/servo/components/plugins/jstraceable.rs b/servo/components/plugins/jstraceable.rs index 82eb3ba3a053..a901f89003ca 100644 --- a/servo/components/plugins/jstraceable.rs +++ b/servo/components/plugins/jstraceable.rs @@ -20,6 +20,9 @@ pub fn expand_dom_struct(_: &mut ExtCtxt, _: Span, _: &MetaItem, item: P) 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|) { 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 { let state_expr = match substr.nonself_args { [ref state_expr] => state_expr, diff --git a/servo/components/plugins/lib.rs b/servo/components/plugins/lib.rs index 2d00c8d3f3bd..2a06fb36f749 100644 --- a/servo/components/plugins/lib.rs +++ b/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; -mod macros; -mod jstraceable; +// Public for documentation to show up +/// Handles the auto-deriving for `#[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)); diff --git a/servo/components/plugins/lints.rs b/servo/components/plugins/lints.rs index fbd229f896e4..aa374cde1d90 100644 --- a/servo/components/plugins/lints.rs +++ b/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` 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 - // Expressions which return out of blocks eventually end up in a `let` + // Catches `let` statements and assignments which store a #[must_root] value + // 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, - _ => return + Some(ref e) => &**e, + _ => 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 }; diff --git a/servo/components/plugins/macros.rs b/servo/components/plugins/macros.rs index 12a1953c43c4..c4807f4dc498 100644 --- a/servo/components/plugins/macros.rs +++ b/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) => ( diff --git a/servo/components/script/dom/xmlhttprequest.rs b/servo/components/script/dom/xmlhttprequest.rs index 3ad488253b0e..869820e0ca6c 100644 --- a/servo/components/script/dom/xmlhttprequest.rs +++ b/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 { - WorkerField(_) if response_type == XMLHttpRequestResponseTypeValues::Document + match self.global.root() { + WorkerRoot(_) if response_type == XMLHttpRequestResponseTypeValues::Document => return Ok(()), _ => {} }