From 4aec737c11f377df90199f0a5605f53600225221 Mon Sep 17 00:00:00 2001 From: Jack Moffitt Date: Mon, 22 Apr 2013 20:13:48 -0600 Subject: [PATCH 1/2] Add unused_unsafe attribute to generated bindings. --- src/servo/dom/bindings/codegen/CodegenRust.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/servo/dom/bindings/codegen/CodegenRust.py b/src/servo/dom/bindings/codegen/CodegenRust.py index fc59fea93b0..71c22234ec5 100644 --- a/src/servo/dom/bindings/codegen/CodegenRust.py +++ b/src/servo/dom/bindings/codegen/CodegenRust.py @@ -2142,7 +2142,7 @@ class CGImports(CGWrapper): # TODO imports to cover descriptors, etc. def _useString(imports): - return '#[allow(unused_imports,unused_variable)];' + ''.join(['use %s;\n' % i for i in imports]) + '\n' + return '#[allow(unused_imports,unused_variable,unused_unsafe)];' + ''.join(['use %s;\n' % i for i in imports]) + '\n' CGWrapper.__init__(self, child, declarePre=_useString(sorted(declareImports))) From 868c7407a85f485a841942bb8195954c3d89a7c7 Mon Sep 17 00:00:00 2001 From: Jack Moffitt Date: Mon, 22 Apr 2013 20:15:46 -0600 Subject: [PATCH 2/2] Remove unneeded unsafe blocks. --- src/servo-gfx/platform/linux/font.rs | 4 +- src/servo-gfx/resource/image_cache_task.rs | 2 +- src/servo-gfx/text/shaping/harfbuzz.rs | 74 ++-- src/servo/content/content_task.rs | 4 +- src/servo/dom/bindings/element.rs | 6 +- src/servo/dom/bindings/utils.rs | 60 ++- src/servo/dom/node.rs | 11 +- src/servo/html/hubbub_html_parser.rs | 406 ++++++++++----------- src/servo/layout/box_builder.rs | 4 +- src/servo/layout/flow.rs | 6 +- 10 files changed, 276 insertions(+), 301 deletions(-) diff --git a/src/servo-gfx/platform/linux/font.rs b/src/servo-gfx/platform/linux/font.rs index 2202885ee3d..288a2cf5876 100644 --- a/src/servo-gfx/platform/linux/font.rs +++ b/src/servo-gfx/platform/linux/font.rs @@ -125,7 +125,7 @@ impl FontHandleMethods for FontHandle { if unsafe { (*self.face).style_flags & FT_STYLE_FLAG_BOLD == 0 } { default_weight } else { - let os2 = unsafe { FT_Get_Sfnt_Table(self.face, ft_sfnt_os2) as *TT_OS2 }; + let os2 = FT_Get_Sfnt_Table(self.face, ft_sfnt_os2) as *TT_OS2; let valid = os2.is_not_null() && unsafe { (*os2).version != 0xffff }; if valid { let weight = unsafe { (*os2).usWeightClass }; @@ -283,7 +283,7 @@ pub impl<'self> FontHandle { // face.size is a *c_void in the bindings, presumably to avoid // recursive structural types let size: &FT_SizeRec = unsafe { cast::transmute(&(*face.size)) }; - let metrics: &FT_Size_Metrics = unsafe { &((*size).metrics) }; + let metrics: &FT_Size_Metrics = &(*size).metrics; let em_size = face.units_per_EM as float; let x_scale = (metrics.x_ppem as float) / em_size as float; diff --git a/src/servo-gfx/resource/image_cache_task.rs b/src/servo-gfx/resource/image_cache_task.rs index f6ad4b2d734..07abb1d05d4 100644 --- a/src/servo-gfx/resource/image_cache_task.rs +++ b/src/servo-gfx/resource/image_cache_task.rs @@ -54,7 +54,7 @@ pub enum ImageResponseMsg { impl ImageResponseMsg { fn clone(&self) -> ImageResponseMsg { match *self { - ImageReady(ref img) => ImageReady(unsafe { clone_arc(img) }), + ImageReady(ref img) => ImageReady(clone_arc(img)), ImageNotReady => ImageNotReady, ImageFailed => ImageFailed } diff --git a/src/servo-gfx/text/shaping/harfbuzz.rs b/src/servo-gfx/text/shaping/harfbuzz.rs index 65e642b4e9e..6cf3ea9ab65 100644 --- a/src/servo-gfx/text/shaping/harfbuzz.rs +++ b/src/servo-gfx/text/shaping/harfbuzz.rs @@ -74,21 +74,19 @@ pub struct ShapedGlyphEntry { impl ShapedGlyphData { pub fn new(buffer: *hb_buffer_t) -> ShapedGlyphData { - unsafe { - let glyph_count = 0; - let glyph_infos = hb_buffer_get_glyph_infos(buffer, &glyph_count); - let glyph_count = glyph_count as uint; - assert!(glyph_infos.is_not_null()); - let pos_count = 0; - let pos_infos = hb_buffer_get_glyph_positions(buffer, &pos_count); - assert!(pos_infos.is_not_null()); - assert!(glyph_count == pos_count as uint); + let glyph_count = 0; + let glyph_infos = hb_buffer_get_glyph_infos(buffer, &glyph_count); + let glyph_count = glyph_count as uint; + assert!(glyph_infos.is_not_null()); + let pos_count = 0; + let pos_infos = hb_buffer_get_glyph_positions(buffer, &pos_count); + assert!(pos_infos.is_not_null()); + assert!(glyph_count == pos_count as uint); - ShapedGlyphData { - count: glyph_count, - glyph_infos: glyph_infos, - pos_infos: pos_infos, - } + ShapedGlyphData { + count: glyph_count, + glyph_infos: glyph_infos, + pos_infos: pos_infos, } } @@ -167,35 +165,33 @@ impl Drop for Shaper { impl Shaper { pub fn new(font: @mut Font) -> Shaper { - unsafe { - let font_ptr: *mut Font = &mut *font; - let hb_face: *hb_face_t = hb_face_create_for_tables(get_font_table_func, - font_ptr as *c_void, - null()); - let hb_font: *hb_font_t = hb_font_create(hb_face); + let font_ptr: *mut Font = &mut *font; + let hb_face: *hb_face_t = hb_face_create_for_tables(get_font_table_func, + font_ptr as *c_void, + null()); + let hb_font: *hb_font_t = hb_font_create(hb_face); - // Set points-per-em. if zero, performs no hinting in that direction. - let pt_size = font.style.pt_size; - hb_font_set_ppem(hb_font, pt_size as c_uint, pt_size as c_uint); + // Set points-per-em. if zero, performs no hinting in that direction. + let pt_size = font.style.pt_size; + hb_font_set_ppem(hb_font, pt_size as c_uint, pt_size as c_uint); - // Set scaling. Note that this takes 16.16 fixed point. - hb_font_set_scale(hb_font, - Shaper::float_to_fixed(pt_size) as c_int, - Shaper::float_to_fixed(pt_size) as c_int); + // Set scaling. Note that this takes 16.16 fixed point. + hb_font_set_scale(hb_font, + Shaper::float_to_fixed(pt_size) as c_int, + Shaper::float_to_fixed(pt_size) as c_int); - // configure static function callbacks. - // NB. This funcs structure could be reused globally, as it never changes. - let hb_funcs: *hb_font_funcs_t = hb_font_funcs_create(); - hb_font_funcs_set_glyph_func(hb_funcs, glyph_func, null(), null()); - hb_font_funcs_set_glyph_h_advance_func(hb_funcs, glyph_h_advance_func, null(), null()); - hb_font_set_funcs(hb_font, hb_funcs, font_ptr as *c_void, null()); + // configure static function callbacks. + // NB. This funcs structure could be reused globally, as it never changes. + let hb_funcs: *hb_font_funcs_t = hb_font_funcs_create(); + hb_font_funcs_set_glyph_func(hb_funcs, glyph_func, null(), null()); + hb_font_funcs_set_glyph_h_advance_func(hb_funcs, glyph_h_advance_func, null(), null()); + hb_font_set_funcs(hb_font, hb_funcs, font_ptr as *c_void, null()); - Shaper { - font: font, - hb_face: hb_face, - hb_font: hb_font, - hb_funcs: hb_funcs, - } + Shaper { + font: font, + hb_face: hb_face, + hb_font: hb_font, + hb_funcs: hb_funcs, } } diff --git a/src/servo/content/content_task.rs b/src/servo/content/content_task.rs index 71857b92ae9..b7518cadafa 100644 --- a/src/servo/content/content_task.rs +++ b/src/servo/content/content_task.rs @@ -156,9 +156,7 @@ pub fn Content(layout_task: LayoutTask, } pub fn task_from_context(cx: *JSContext) -> *mut Content { - unsafe { - JS_GetContextPrivate(cx) as *mut Content - } + JS_GetContextPrivate(cx) as *mut Content } #[allow(non_implicitly_copyable_typarams)] diff --git a/src/servo/dom/bindings/element.rs b/src/servo/dom/bindings/element.rs index 207e98b2950..9270375030d 100644 --- a/src/servo/dom/bindings/element.rs +++ b/src/servo/dom/bindings/element.rs @@ -202,10 +202,8 @@ pub fn create(cx: *JSContext, node: &mut AbstractNode) -> jsobj { node.get_wrappercache().set_wrapper(obj.ptr); - unsafe { - let raw_ptr = ptr::addr_of(node) as *libc::c_void; - JS_SetReservedSlot(obj.ptr, DOM_OBJECT_SLOT as u32, RUST_PRIVATE_TO_JSVAL(raw_ptr)); - } + let raw_ptr = ptr::addr_of(node) as *libc::c_void; + JS_SetReservedSlot(obj.ptr, DOM_OBJECT_SLOT as u32, RUST_PRIVATE_TO_JSVAL(raw_ptr)); return obj; } diff --git a/src/servo/dom/bindings/utils.rs b/src/servo/dom/bindings/utils.rs index 0800a719166..c2cbd671483 100644 --- a/src/servo/dom/bindings/utils.rs +++ b/src/servo/dom/bindings/utils.rs @@ -372,38 +372,36 @@ pub fn CreateInterfaceObjects2(cx: *JSContext, global: *JSObject, receiver: *JSO constants: *ConstantSpec, staticMethods: *JSFunctionSpec, name: &str) -> *JSObject { - unsafe { - let mut proto = ptr::null(); - if protoClass.is_not_null() { - proto = CreateInterfacePrototypeObject(cx, global, protoProto, - protoClass, methods, - properties, constants); - if proto.is_null() { - return ptr::null(); - } - - JS_SetReservedSlot(proto, DOM_PROTO_INSTANCE_CLASS_SLOT, - RUST_PRIVATE_TO_JSVAL(domClass as *libc::c_void)); + let mut proto = ptr::null(); + if protoClass.is_not_null() { + proto = CreateInterfacePrototypeObject(cx, global, protoProto, + protoClass, methods, + properties, constants); + if proto.is_null() { + return ptr::null(); } - let mut interface = ptr::null(); - if constructorClass.is_not_null() || constructor.is_not_null() { - interface = do str::as_c_str(name) |s| { - CreateInterfaceObject(cx, global, receiver, constructorClass, - constructor, ctorNargs, proto, - staticMethods, constants, s) - }; - if interface.is_null() { - return ptr::null(); - } - } + JS_SetReservedSlot(proto, DOM_PROTO_INSTANCE_CLASS_SLOT, + RUST_PRIVATE_TO_JSVAL(domClass as *libc::c_void)); + } - if protoClass.is_not_null() { - proto - } else { - interface + let mut interface = ptr::null(); + if constructorClass.is_not_null() || constructor.is_not_null() { + interface = do str::as_c_str(name) |s| { + CreateInterfaceObject(cx, global, receiver, constructorClass, + constructor, ctorNargs, proto, + staticMethods, constants, s) + }; + if interface.is_null() { + return ptr::null(); } } + + if protoClass.is_not_null() { + proto + } else { + interface + } } fn CreateInterfaceObject(cx: *JSContext, global: *JSObject, receiver: *JSObject, @@ -412,7 +410,6 @@ fn CreateInterfaceObject(cx: *JSContext, global: *JSObject, receiver: *JSObject, staticMethods: *JSFunctionSpec, constants: *ConstantSpec, name: *libc::c_char) -> *JSObject { - unsafe { let constructor = if constructorClass.is_not_null() { let functionProto = JS_GetFunctionPrototype(cx, global); if functionProto.is_null() { @@ -482,7 +479,6 @@ fn CreateInterfaceObject(cx: *JSContext, global: *JSObject, receiver: *JSObject, } return constructor; - } } fn DefineConstants(cx: *JSContext, obj: *JSObject, constants: *ConstantSpec) -> bool { @@ -506,11 +502,11 @@ fn DefineConstants(cx: *JSContext, obj: *JSObject, constants: *ConstantSpec) -> } fn DefineMethods(cx: *JSContext, obj: *JSObject, methods: *JSFunctionSpec) -> bool { - unsafe { JS_DefineFunctions(cx, obj, methods) != 0 } + JS_DefineFunctions(cx, obj, methods) != 0 } fn DefineProperties(cx: *JSContext, obj: *JSObject, properties: *JSPropertySpec) -> bool { - unsafe { JS_DefineProperties(cx, obj, properties) != 0 } + JS_DefineProperties(cx, obj, properties) != 0 } fn CreateInterfacePrototypeObject(cx: *JSContext, global: *JSObject, @@ -571,7 +567,7 @@ pub impl WrapperCache { } fn set_wrapper(&mut self, wrapper: *JSObject) { - unsafe { self.wrapper = wrapper; } + self.wrapper = wrapper; } fn new() -> WrapperCache { diff --git a/src/servo/dom/node.rs b/src/servo/dom/node.rs index e7e0d7466c0..f1903892e0a 100644 --- a/src/servo/dom/node.rs +++ b/src/servo/dom/node.rs @@ -335,18 +335,13 @@ impl DebugMethods for AbstractNode { debug!("%s", s); // FIXME: this should have a pure version? - unsafe { - for self.each_child() |kid| { - kid.dump_indent(indent + 1u) - } + for self.each_child() |kid| { + kid.dump_indent(indent + 1u) } } fn debug_str(&self) -> ~str { - // Unsafe due to the call to type_id(). - unsafe { - fmt!("%?", self.type_id()) - } + fmt!("%?", self.type_id()) } } diff --git a/src/servo/html/hubbub_html_parser.rs b/src/servo/html/hubbub_html_parser.rs index 7dfdae3cc9a..27146f49d67 100644 --- a/src/servo/html/hubbub_html_parser.rs +++ b/src/servo/html/hubbub_html_parser.rs @@ -224,217 +224,211 @@ pub fn parse_html(url: Url, let url2 = url.clone(), url3 = url.clone(); - unsafe { - // Build the root node. - let root = ~HTMLHtmlElement { parent: Element::new(HTMLHtmlElementTypeId, ~"html") }; - let root = unsafe { Node::as_abstract_node(root) }; - debug!("created new node"); - let mut parser = hubbub::Parser("UTF-8", false); - debug!("created parser"); - parser.set_document_node(root.to_hubbub_node()); - parser.enable_scripting(true); + // Build the root node. + let root = ~HTMLHtmlElement { parent: Element::new(HTMLHtmlElementTypeId, ~"html") }; + let root = unsafe { Node::as_abstract_node(root) }; + debug!("created new node"); + let mut parser = hubbub::Parser("UTF-8", false); + debug!("created parser"); + parser.set_document_node(root.to_hubbub_node()); + parser.enable_scripting(true); - // Performs various actions necessary after appending has taken place. Currently, this - // consists of processing inline stylesheets, but in the future it might perform - // prefetching, etc. - let css_chan2 = css_chan.clone(); - let append_hook: ~fn(AbstractNode, AbstractNode) = |parent_node, child_node| { - if parent_node.is_style_element() && child_node.is_text() { - debug!("found inline CSS stylesheet"); - let url = url::from_str("http://example.com/"); // FIXME - let url_cell = Cell(url); - do child_node.with_imm_text |text_node| { - let data = text_node.text.to_str(); // FIXME: Bad copy. - let provenance = InlineProvenance(result::unwrap(url_cell.take()), data); - css_chan2.send(CSSTaskNewFile(provenance)); - } - } - }; - - let (css_chan2, js_chan2) = (css_chan.clone(), js_chan.clone()); - parser.set_tree_handler(~hubbub::TreeHandler { - create_comment: |data: ~str| { - debug!("create comment"); - unsafe { - Node::as_abstract_node(~Comment::new(data)).to_hubbub_node() - } - }, - create_doctype: |doctype: ~hubbub::Doctype| { - debug!("create doctype"); - // TODO: remove copying here by using struct pattern matching to - // move all ~strs at once (blocked on Rust #3845, #3846, #3847) - let public_id = match &doctype.public_id { - &None => None, - &Some(ref id) => Some(copy *id) - }; - let system_id = match &doctype.system_id { - &None => None, - &Some(ref id) => Some(copy *id) - }; - let node = ~Doctype::new(copy doctype.name, - public_id, - system_id, - doctype.force_quirks); - unsafe { - Node::as_abstract_node(node).to_hubbub_node() - } - }, - create_element: |tag: ~hubbub::Tag| { - debug!("create element"); - // TODO: remove copying here by using struct pattern matching to - // move all ~strs at once (blocked on Rust #3845, #3846, #3847) - let node = build_element_from_tag(tag.name); - - debug!("-- attach attrs"); - do node.as_mut_element |element| { - for tag.attributes.each |attr| { - element.attrs.push(Attr::new(copy attr.name, copy attr.value)); - } - } - - // Spawn additional parsing, network loads, etc. from tag and attrs - match node.type_id() { - // Handle CSS style sheets from elements - ElementNodeTypeId(HTMLLinkElementTypeId) => { - do node.with_imm_element |element| { - match (element.get_attr(~"rel"), element.get_attr(~"href")) { - (Some(rel), Some(href)) => { - if rel == ~"stylesheet" { - debug!("found CSS stylesheet: %s", href); - let url = make_url(href.to_str(), Some(url2.clone())); - css_chan2.send(CSSTaskNewFile(UrlProvenance(url))); - } - } - _ => {} - } - } - }, - ElementNodeTypeId(HTMLImageElementTypeId) => { - do node.with_mut_image_element |image_element| { - let src_opt = image_element.parent.get_attr(~"src").map(|x| x.to_str()); - match src_opt { - None => {} - Some(src) => { - let img_url = make_url(src, Some(url2.clone())); - image_element.image = Some(copy img_url); - // inform the image cache to load this, but don't store a handle. - // TODO (Issue #84): don't prefetch if we are within a