Bug 1709509 - Fix clippy warnings in webrender_bindings. r=gfx-reviewers,lsalzman

Fixed the following:
- methods called `to_*` usually take self by reference; consider choosing
  a less ambiguous name (clippy::wrong_self_convention)
- an implementation of `From` is preferred since it gives you `Into<_>`
  for free where the reverse isn't true (clippy::from_over_into)
- manual implementation of `Option::map` (clippy::manual_map)
- comparing with null is better expressed by the `.is_null()` method
  (clippy::cmp_null)
- use of `unwrap_or` followed by a function call (clippy::or_fun_call)
- you don't need to add `&` to all patterns (clippy::match_ref_pats)
- writing `&PathBuf` instead of `&Path` involves a new object where a
  slice will do (clippy::ptr_arg)

Locally ignored:
- 6 bindings with single-character names in scope (clippy::many_single_char_names)

Globally ignored:
- this public function dereferences a raw pointer but is not marked
  `unsafe` (clippy::not_unsafe_ptr_arg_deref)
- unsafe function's docs miss `# Safety` section (clippy::missing_safety_doc)

Differential Revision: https://phabricator.services.mozilla.com/D114319
This commit is contained in:
Mike Hommey 2021-05-05 05:50:32 +00:00
parent 40dabb3533
commit 4178828587
4 changed files with 26 additions and 31 deletions

View File

@ -2,6 +2,9 @@
* 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/. */
#![allow(clippy::missing_safety_doc)]
#![allow(clippy::not_unsafe_ptr_arg_deref)]
use gleam::gl; use gleam::gl;
use std::cell::RefCell; use std::cell::RefCell;
#[cfg(not(target_os = "macos"))] #[cfg(not(target_os = "macos"))]
@ -240,11 +243,11 @@ pub struct WrVecU8 {
} }
impl WrVecU8 { impl WrVecU8 {
fn to_vec(self) -> Vec<u8> { fn into_vec(self) -> Vec<u8> {
unsafe { Vec::from_raw_parts(self.data, self.length, self.capacity) } unsafe { Vec::from_raw_parts(self.data, self.length, self.capacity) }
} }
// Equivalent to `to_vec` but clears self instead of consuming the value. // Equivalent to `into_vec` but clears self instead of consuming the value.
fn flush_into_vec(&mut self) -> Vec<u8> { fn flush_into_vec(&mut self) -> Vec<u8> {
self.convert_into_vec::<u8>() self.convert_into_vec::<u8>()
} }
@ -299,7 +302,7 @@ pub extern "C" fn wr_vec_u8_reserve(v: &mut WrVecU8, len: usize) {
#[no_mangle] #[no_mangle]
pub extern "C" fn wr_vec_u8_free(v: WrVecU8) { pub extern "C" fn wr_vec_u8_free(v: WrVecU8) {
v.to_vec(); v.into_vec();
} }
#[repr(C)] #[repr(C)]
@ -357,18 +360,18 @@ pub struct WrImageDescriptor {
pub prefer_compositor_surface: bool, pub prefer_compositor_surface: bool,
} }
impl<'a> Into<ImageDescriptor> for &'a WrImageDescriptor { impl<'a> From<&'a WrImageDescriptor> for ImageDescriptor {
fn into(self) -> ImageDescriptor { fn from(desc: &'a WrImageDescriptor) -> ImageDescriptor {
let mut flags = ImageDescriptorFlags::empty(); let mut flags = ImageDescriptorFlags::empty();
if self.opacity == OpacityType::Opaque { if desc.opacity == OpacityType::Opaque {
flags |= ImageDescriptorFlags::IS_OPAQUE; flags |= ImageDescriptorFlags::IS_OPAQUE;
} }
ImageDescriptor { ImageDescriptor {
size: DeviceIntSize::new(self.width, self.height), size: DeviceIntSize::new(desc.width, desc.height),
stride: if self.stride != 0 { Some(self.stride) } else { None }, stride: if desc.stride != 0 { Some(desc.stride) } else { None },
format: self.format, format: desc.format,
offset: 0, offset: 0,
flags, flags,
} }
@ -1173,10 +1176,7 @@ fn wr_device_new(gl_context: *mut c_void, pc: Option<&mut WrProgramCache>) -> De
let use_optimized_shaders = unsafe { gfx_wr_use_optimized_shaders() }; let use_optimized_shaders = unsafe { gfx_wr_use_optimized_shaders() };
let cached_programs = match pc { let cached_programs = pc.map(|cached_programs| Rc::clone(cached_programs.rc_get()));
Some(cached_programs) => Some(Rc::clone(cached_programs.rc_get())),
None => None,
};
Device::new( Device::new(
gl, gl,
@ -1419,7 +1419,7 @@ impl MappableCompositor for WrCompositor {
); );
} }
if tile_info.data != ptr::null_mut() && tile_info.stride != 0 { if !tile_info.data.is_null() && tile_info.stride != 0 {
Some(tile_info) Some(tile_info)
} else { } else {
None None
@ -1499,14 +1499,14 @@ pub extern "C" fn wr_window_new(
) -> bool { ) -> bool {
assert!(unsafe { is_in_render_thread() }); assert!(unsafe { is_in_render_thread() });
let software = swgl_context != ptr::null_mut(); let software = !swgl_context.is_null();
let (gl, sw_gl) = if software { let (gl, sw_gl) = if software {
let ctx = swgl::Context::from(swgl_context); let ctx = swgl::Context::from(swgl_context);
ctx.make_current(); ctx.make_current();
(Rc::new(ctx) as Rc<dyn gl::Gl>, Some(ctx)) (Rc::new(ctx) as Rc<dyn gl::Gl>, Some(ctx))
} else { } else {
let gl = unsafe { let gl = unsafe {
if gl_context == ptr::null_mut() { if gl_context.is_null() {
panic!("Native GL context required when not using SWGL!"); panic!("Native GL context required when not using SWGL!");
} else if is_glcontext_gles(gl_context) { } else if is_glcontext_gles(gl_context) {
gl::GlesFns::load_with(|symbol| get_proc_address(gl_context, symbol)) gl::GlesFns::load_with(|symbol| get_proc_address(gl_context, symbol))
@ -1530,7 +1530,7 @@ pub extern "C" fn wr_window_new(
} }
}; };
let upload_method = if gl_context != ptr::null_mut() && unsafe { is_glcontext_angle(gl_context) } { let upload_method = if !gl_context.is_null() && unsafe { is_glcontext_angle(gl_context) } {
UploadMethod::Immediate UploadMethod::Immediate
} else { } else {
UploadMethod::PixelBuffer(ONE_TIME_USAGE_HINT) UploadMethod::PixelBuffer(ONE_TIME_USAGE_HINT)
@ -1542,10 +1542,7 @@ pub extern "C" fn wr_window_new(
ShaderPrecacheFlags::empty() ShaderPrecacheFlags::empty()
}; };
let cached_programs = match program_cache { let cached_programs = program_cache.map(|program_cache| Rc::clone(&program_cache.rc_get()));
Some(program_cache) => Some(Rc::clone(&program_cache.rc_get())),
None => None,
};
let color = if cfg!(target_os = "android") { let color = if cfg!(target_os = "android") {
// The color is for avoiding black flash before receiving display list. // The color is for avoiding black flash before receiving display list.
@ -1684,7 +1681,7 @@ pub extern "C" fn wr_window_new(
#[no_mangle] #[no_mangle]
pub unsafe extern "C" fn wr_api_free_error_msg(msg: *mut c_char) { pub unsafe extern "C" fn wr_api_free_error_msg(msg: *mut c_char) {
if msg != ptr::null_mut() { if !msg.is_null() {
CString::from_raw(msg); CString::from_raw(msg);
} }
} }
@ -2489,10 +2486,7 @@ pub extern "C" fn wr_dp_push_stacking_context(
.collect(); .collect();
let transform_ref = unsafe { transform.as_ref() }; let transform_ref = unsafe { transform.as_ref() };
let mut transform_binding = match transform_ref { let mut transform_binding = transform_ref.map(|t| PropertyBinding::Value(*t));
Some(t) => Some(PropertyBinding::Value(*t)),
None => None,
};
let computed_ref = unsafe { params.computed_transform.as_ref() }; let computed_ref = unsafe { params.computed_transform.as_ref() };
let opacity_ref = unsafe { params.opacity.as_ref() }; let opacity_ref = unsafe { params.opacity.as_ref() };
@ -2519,7 +2513,7 @@ pub extern "C" fn wr_dp_push_stacking_context(
transform_binding = Some(PropertyBinding::Binding( transform_binding = Some(PropertyBinding::Binding(
PropertyBindingKey::new(anim.id), PropertyBindingKey::new(anim.id),
// Same as above opacity case. // Same as above opacity case.
transform_ref.cloned().unwrap_or(LayoutTransform::identity()), transform_ref.cloned().unwrap_or_else(LayoutTransform::identity),
)); ));
} }
_ => unreachable!("{:?} should not create a stacking context", anim.effect_type), _ => unreachable!("{:?} should not create a stacking context", anim.effect_type),

View File

@ -853,10 +853,10 @@ impl Moz2dBlobImageHandler {
if !unsafe { HasFontData(instance.font_key) } { if !unsafe { HasFontData(instance.font_key) } {
let template = resources.get_font_data(instance.font_key); let template = resources.get_font_data(instance.font_key);
match template { match template {
&FontTemplate::Raw(ref data, ref index) => unsafe { FontTemplate::Raw(ref data, ref index) => unsafe {
AddFontData(instance.font_key, data.as_ptr(), data.len(), *index, data); AddFontData(instance.font_key, data.as_ptr(), data.len(), *index, data);
}, },
&FontTemplate::Native(ref handle) => { FontTemplate::Native(ref handle) => {
process_native_font_handle(instance.font_key, handle); process_native_font_handle(instance.font_key, handle);
} }
} }

View File

@ -7,7 +7,7 @@ use std::ffi::OsString;
use std::fs::{create_dir_all, read_dir, read_to_string, File}; use std::fs::{create_dir_all, read_dir, read_to_string, File};
use std::io::{Error, ErrorKind}; use std::io::{Error, ErrorKind};
use std::io::{Read, Write}; use std::io::{Read, Write};
use std::path::PathBuf; use std::path::{PathBuf, Path};
use std::rc::Rc; use std::rc::Rc;
use std::sync::Arc; use std::sync::Arc;
@ -19,7 +19,7 @@ use webrender::{ProgramBinary, ProgramCache, ProgramCacheObserver, ProgramSource
const MAX_LOAD_TIME_MS: u64 = 400; const MAX_LOAD_TIME_MS: u64 = 400;
fn deserialize_program_binary(path: &PathBuf) -> Result<Arc<ProgramBinary>, Error> { fn deserialize_program_binary(path: &Path) -> Result<Arc<ProgramBinary>, Error> {
let mut buf = vec![]; let mut buf = vec![];
let mut file = File::open(path)?; let mut file = File::open(path)?;
file.read_to_end(&mut buf)?; file.read_to_end(&mut buf)?;

View File

@ -84,6 +84,7 @@ pub extern "C" fn wr_swgl_set_texture_buffer(
} }
#[no_mangle] #[no_mangle]
#[allow(clippy::many_single_char_names)]
pub extern "C" fn wr_swgl_clear_color_rect( pub extern "C" fn wr_swgl_clear_color_rect(
ctx: *mut c_void, ctx: *mut c_void,
fbo: u32, fbo: u32,