From d25bc49772a4a5ca31e0996cdde687faf72143e1 Mon Sep 17 00:00:00 2001 From: Chris Manchester Date: Wed, 26 Nov 2014 20:02:35 -0500 Subject: [PATCH] Return early and decline analysis when linting within unsafe functions for must_root analysis. Removes a handful of whitelist annotations obsoleted by this change. fixes #3658 --- components/plugins/lints.rs | 54 ++++++++++++++++++++++++++++++-- components/script/dom/element.rs | 9 ------ 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/components/plugins/lints.rs b/components/plugins/lints.rs index f686623d190..a467ae7c91f 100644 --- a/components/plugins/lints.rs +++ b/components/plugins/lints.rs @@ -2,7 +2,7 @@ * 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/. */ -use syntax::{ast, ast_util, codemap, visit}; +use syntax::{ast, ast_map, ast_util, codemap, visit}; use syntax::ast::Public; use syntax::attr::AttrMetaMethods; use rustc::lint::{Context, LintPass, LintArray}; @@ -92,6 +92,35 @@ fn lint_unrooted_ty(cx: &Context, ty: &ast::Ty, warning: &str) { }; } +// Determines if a block is in an unsafe context so that an unhelpful +// lint can be aborted. +fn unsafe_context(map: &ast_map::Map, id: ast::NodeId) -> bool { + match map.get(map.get_parent(id)) { + ast_map::NodeImplItem(itm) => { + match *itm { + ast::MethodImplItem(ref meth) => match meth.node { + ast::MethDecl(_, _, _, _, style, _, _, _) => match style { + ast::UnsafeFn => true, + _ => false, + }, + _ => false, + }, + _ => false, + } + }, + ast_map::NodeItem(itm) => { + match itm.node { + ast::ItemFn(_, style, _, _, _) => match style { + ast::UnsafeFn => true, + _ => false, + }, + _ => false, + } + } + _ => false // There are probably a couple of other unsafe cases we don't care to lint, those will need to be added. + } +} + impl LintPass for UnrootedPass { fn get_lints(&self) -> LintArray { lint_array!(UNROOTED_MUST_ROOT) @@ -122,14 +151,23 @@ 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) { + 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" => { return; - } + }, + visit::FkItemFn(_, _, style, _) => match style { + ast::UnsafeFn => return, + _ => () + }, _ => () } + + if unsafe_context(&cx.tcx.map, id) { + return; + } + match block.rules { ast::DefaultBlock => { for arg in decl.inputs.iter() { @@ -146,6 +184,16 @@ 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) | + ast::StmtSemi(_, id) if unsafe_context(&cx.tcx.map, id) => { + return + }, + _ => () + }; + let expr = match s.node { // Catch a `let` binding ast::StmtDecl(ref decl, _) => match decl.node { diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index d3b5b1946aa..78c06a410ab 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -201,7 +201,6 @@ pub trait RawLayoutElementHelpers { } #[inline] -#[allow(unrooted_must_root)] unsafe fn get_attr_for_layout<'a>(elem: &'a Element, namespace: &Namespace, name: &Atom) -> Option<&'a JS> { // cast to point to T in RefCell directly let attrs: *const Vec> = mem::transmute(&elem.attrs); @@ -214,7 +213,6 @@ unsafe fn get_attr_for_layout<'a>(elem: &'a Element, namespace: &Namespace, name impl RawLayoutElementHelpers for Element { #[inline] - #[allow(unrooted_must_root)] unsafe fn get_attr_val_for_layout<'a>(&'a self, namespace: &Namespace, name: &Atom) -> Option<&'a str> { get_attr_for_layout(self, namespace, name).map(|attr| { @@ -224,7 +222,6 @@ impl RawLayoutElementHelpers for Element { } #[inline] - #[allow(unrooted_must_root)] unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &Atom) -> Vec<&'a str> { let attrs = self.attrs.borrow_for_layout(); (*attrs).iter().filter_map(|attr: &JS| { @@ -238,7 +235,6 @@ impl RawLayoutElementHelpers for Element { } #[inline] - #[allow(unrooted_must_root)] unsafe fn get_attr_atom_for_layout(&self, namespace: &Namespace, name: &Atom) -> Option { let attrs = self.attrs.borrow_for_layout(); @@ -253,7 +249,6 @@ impl RawLayoutElementHelpers for Element { } #[inline] - #[allow(unrooted_must_root)] unsafe fn has_class_for_layout(&self, name: &Atom) -> bool { let attrs = self.attrs.borrow_for_layout(); (*attrs).iter().find(|attr: & &JS| { @@ -268,7 +263,6 @@ impl RawLayoutElementHelpers for Element { } #[inline] - #[allow(unrooted_must_root)] unsafe fn get_classes_for_layout(&self) -> Option<&'static [Atom]> { let attrs = self.attrs.borrow_for_layout(); (*attrs).iter().find(|attr: & &JS| { @@ -281,7 +275,6 @@ impl RawLayoutElementHelpers for Element { } #[inline] - #[allow(unrooted_must_root)] unsafe fn get_length_attribute_for_layout(&self, length_attribute: LengthAttribute) -> LengthOrPercentageOrAuto { match length_attribute { @@ -296,7 +289,6 @@ impl RawLayoutElementHelpers for Element { } #[inline] - #[allow(unrooted_must_root)] unsafe fn get_integer_attribute_for_layout(&self, integer_attribute: IntegerAttribute) -> Option { match integer_attribute { @@ -331,7 +323,6 @@ pub trait LayoutElementHelpers { } impl LayoutElementHelpers for JS { - #[allow(unrooted_must_root)] #[inline] unsafe fn html_element_in_html_document_for_layout(&self) -> bool { if (*self.unsafe_get()).namespace != ns!(HTML) {