mirror of
https://github.com/servo/servo.git
synced 2025-08-05 21:50:18 +01:00
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
This commit is contained in:
parent
c91e949ed0
commit
d25bc49772
2 changed files with 51 additions and 12 deletions
|
@ -2,7 +2,7 @@
|
||||||
* License, v. 2.0. If a copy of the MPL was not distributed with this
|
* 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/. */
|
* 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::ast::Public;
|
||||||
use syntax::attr::AttrMetaMethods;
|
use syntax::attr::AttrMetaMethods;
|
||||||
use rustc::lint::{Context, LintPass, LintArray};
|
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 {
|
impl LintPass for UnrootedPass {
|
||||||
fn get_lints(&self) -> LintArray {
|
fn get_lints(&self) -> LintArray {
|
||||||
lint_array!(UNROOTED_MUST_ROOT)
|
lint_array!(UNROOTED_MUST_ROOT)
|
||||||
|
@ -122,14 +151,23 @@ impl LintPass for UnrootedPass {
|
||||||
}
|
}
|
||||||
/// Function arguments that are #[must_root] types are not allowed
|
/// Function arguments that are #[must_root] types are not allowed
|
||||||
fn check_fn(&mut self, cx: &Context, kind: visit::FnKind, decl: &ast::FnDecl,
|
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 {
|
match kind {
|
||||||
visit::FkItemFn(i, _, _, _) |
|
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" => {
|
||||||
return;
|
return;
|
||||||
}
|
},
|
||||||
|
visit::FkItemFn(_, _, style, _) => match style {
|
||||||
|
ast::UnsafeFn => return,
|
||||||
|
_ => ()
|
||||||
|
},
|
||||||
_ => ()
|
_ => ()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if unsafe_context(&cx.tcx.map, id) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
match block.rules {
|
match block.rules {
|
||||||
ast::DefaultBlock => {
|
ast::DefaultBlock => {
|
||||||
for arg in decl.inputs.iter() {
|
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
|
// 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)
|
// statement or a function return (which will be caught when it is used elsewhere)
|
||||||
fn check_stmt(&mut self, cx: &Context, s: &ast::Stmt) {
|
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 {
|
let expr = match s.node {
|
||||||
// Catch a `let` binding
|
// Catch a `let` binding
|
||||||
ast::StmtDecl(ref decl, _) => match decl.node {
|
ast::StmtDecl(ref decl, _) => match decl.node {
|
||||||
|
|
|
@ -201,7 +201,6 @@ pub trait RawLayoutElementHelpers {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[inline]
|
#[inline]
|
||||||
#[allow(unrooted_must_root)]
|
|
||||||
unsafe fn get_attr_for_layout<'a>(elem: &'a Element, namespace: &Namespace, name: &Atom) -> Option<&'a JS<Attr>> {
|
unsafe fn get_attr_for_layout<'a>(elem: &'a Element, namespace: &Namespace, name: &Atom) -> Option<&'a JS<Attr>> {
|
||||||
// cast to point to T in RefCell<T> directly
|
// cast to point to T in RefCell<T> directly
|
||||||
let attrs: *const Vec<JS<Attr>> = mem::transmute(&elem.attrs);
|
let attrs: *const Vec<JS<Attr>> = 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 {
|
impl RawLayoutElementHelpers for Element {
|
||||||
#[inline]
|
#[inline]
|
||||||
#[allow(unrooted_must_root)]
|
|
||||||
unsafe fn get_attr_val_for_layout<'a>(&'a self, namespace: &Namespace, name: &Atom)
|
unsafe fn get_attr_val_for_layout<'a>(&'a self, namespace: &Namespace, name: &Atom)
|
||||||
-> Option<&'a str> {
|
-> Option<&'a str> {
|
||||||
get_attr_for_layout(self, namespace, name).map(|attr| {
|
get_attr_for_layout(self, namespace, name).map(|attr| {
|
||||||
|
@ -224,7 +222,6 @@ impl RawLayoutElementHelpers for Element {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[inline]
|
#[inline]
|
||||||
#[allow(unrooted_must_root)]
|
|
||||||
unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &Atom) -> Vec<&'a str> {
|
unsafe fn get_attr_vals_for_layout<'a>(&'a self, name: &Atom) -> Vec<&'a str> {
|
||||||
let attrs = self.attrs.borrow_for_layout();
|
let attrs = self.attrs.borrow_for_layout();
|
||||||
(*attrs).iter().filter_map(|attr: &JS<Attr>| {
|
(*attrs).iter().filter_map(|attr: &JS<Attr>| {
|
||||||
|
@ -238,7 +235,6 @@ impl RawLayoutElementHelpers for Element {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[inline]
|
#[inline]
|
||||||
#[allow(unrooted_must_root)]
|
|
||||||
unsafe fn get_attr_atom_for_layout(&self, namespace: &Namespace, name: &Atom)
|
unsafe fn get_attr_atom_for_layout(&self, namespace: &Namespace, name: &Atom)
|
||||||
-> Option<Atom> {
|
-> Option<Atom> {
|
||||||
let attrs = self.attrs.borrow_for_layout();
|
let attrs = self.attrs.borrow_for_layout();
|
||||||
|
@ -253,7 +249,6 @@ impl RawLayoutElementHelpers for Element {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[inline]
|
#[inline]
|
||||||
#[allow(unrooted_must_root)]
|
|
||||||
unsafe fn has_class_for_layout(&self, name: &Atom) -> bool {
|
unsafe fn has_class_for_layout(&self, name: &Atom) -> bool {
|
||||||
let attrs = self.attrs.borrow_for_layout();
|
let attrs = self.attrs.borrow_for_layout();
|
||||||
(*attrs).iter().find(|attr: & &JS<Attr>| {
|
(*attrs).iter().find(|attr: & &JS<Attr>| {
|
||||||
|
@ -268,7 +263,6 @@ impl RawLayoutElementHelpers for Element {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[inline]
|
#[inline]
|
||||||
#[allow(unrooted_must_root)]
|
|
||||||
unsafe fn get_classes_for_layout(&self) -> Option<&'static [Atom]> {
|
unsafe fn get_classes_for_layout(&self) -> Option<&'static [Atom]> {
|
||||||
let attrs = self.attrs.borrow_for_layout();
|
let attrs = self.attrs.borrow_for_layout();
|
||||||
(*attrs).iter().find(|attr: & &JS<Attr>| {
|
(*attrs).iter().find(|attr: & &JS<Attr>| {
|
||||||
|
@ -281,7 +275,6 @@ impl RawLayoutElementHelpers for Element {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[inline]
|
#[inline]
|
||||||
#[allow(unrooted_must_root)]
|
|
||||||
unsafe fn get_length_attribute_for_layout(&self, length_attribute: LengthAttribute)
|
unsafe fn get_length_attribute_for_layout(&self, length_attribute: LengthAttribute)
|
||||||
-> LengthOrPercentageOrAuto {
|
-> LengthOrPercentageOrAuto {
|
||||||
match length_attribute {
|
match length_attribute {
|
||||||
|
@ -296,7 +289,6 @@ impl RawLayoutElementHelpers for Element {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[inline]
|
#[inline]
|
||||||
#[allow(unrooted_must_root)]
|
|
||||||
unsafe fn get_integer_attribute_for_layout(&self, integer_attribute: IntegerAttribute)
|
unsafe fn get_integer_attribute_for_layout(&self, integer_attribute: IntegerAttribute)
|
||||||
-> Option<i32> {
|
-> Option<i32> {
|
||||||
match integer_attribute {
|
match integer_attribute {
|
||||||
|
@ -331,7 +323,6 @@ pub trait LayoutElementHelpers {
|
||||||
}
|
}
|
||||||
|
|
||||||
impl LayoutElementHelpers for JS<Element> {
|
impl LayoutElementHelpers for JS<Element> {
|
||||||
#[allow(unrooted_must_root)]
|
|
||||||
#[inline]
|
#[inline]
|
||||||
unsafe fn html_element_in_html_document_for_layout(&self) -> bool {
|
unsafe fn html_element_in_html_document_for_layout(&self) -> bool {
|
||||||
if (*self.unsafe_get()).namespace != ns!(HTML) {
|
if (*self.unsafe_get()).namespace != ns!(HTML) {
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue