From b8cf5090cbbd26fe3e234e6940e933ad837bd606 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 11 Jun 2022 20:17:37 +0100 Subject: [PATCH 01/11] Make clippy warnings and let CI fail if there are issues --- agb/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/agb/src/lib.rs b/agb/src/lib.rs index 739e01d..aee34b4 100644 --- a/agb/src/lib.rs +++ b/agb/src/lib.rs @@ -5,6 +5,7 @@ #![cfg_attr(test, test_runner(crate::test_runner::test_runner))] #![cfg_attr(test, reexport_test_harness_main = "test_main")] #![feature(alloc_error_handler)] +#![warn(clippy::all)] //! # agb //! `agb` is a library for making games on the Game Boy Advance using the Rust From 1edd7f416604cf56227fe5e2010046e5a55bf6e0 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 11 Jun 2022 20:27:08 +0100 Subject: [PATCH 02/11] Add a bunch of new clippy lints --- agb/src/lib.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/agb/src/lib.rs b/agb/src/lib.rs index aee34b4..87854be 100644 --- a/agb/src/lib.rs +++ b/agb/src/lib.rs @@ -6,6 +6,15 @@ #![cfg_attr(test, reexport_test_harness_main = "test_main")] #![feature(alloc_error_handler)] #![warn(clippy::all)] +#![warn(clippy::must_use_candidate)] +#![warn(clippy::trivially_copy_pass_by_ref)] +#![warn(clippy::semicolon_if_nothing_returned)] +#![warn(clippy::match_same_arms)] +#![warn(clippy::cast_lossless)] +#![warn(clippy::map_unwrap_or)] +#![warn(clippy::needless_pass_by_value)] +#![warn(clippy::redundant_closure_for_method_calls)] +#![warn(clippy::cloned_instead_of_copied)] //! # agb //! `agb` is a library for making games on the Game Boy Advance using the Rust From f2e565f512080ae29e8595ddc9a833b5d4c4705f Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 11 Jun 2022 20:33:32 +0100 Subject: [PATCH 03/11] Take advantage of the fact that CriticalSection is Copy --- agb/examples/output.rs | 4 ++-- agb/examples/wave.rs | 6 +++--- agb/src/agb_alloc/block_allocator.rs | 10 +++++----- agb/src/agb_alloc/bump_allocator.rs | 8 ++++---- agb/src/display/object.rs | 4 ++-- agb/src/hash_map.rs | 4 ++-- agb/src/interrupt.rs | 28 ++++++++++++++-------------- agb/src/rng.rs | 2 +- agb/src/sound/mixer/sw_mixer.rs | 8 ++++---- 9 files changed, 37 insertions(+), 37 deletions(-) diff --git a/agb/examples/output.rs b/agb/examples/output.rs index 6101438..88b6051 100644 --- a/agb/examples/output.rs +++ b/agb/examples/output.rs @@ -10,8 +10,8 @@ fn main(_gba: agb::Gba) -> ! { let count = Mutex::new(RefCell::new(0)); let _a = agb::interrupt::add_interrupt_handler( agb::interrupt::Interrupt::VBlank, - |key: &CriticalSection| { - let mut count = count.borrow(*key).borrow_mut(); + |key: CriticalSection| { + let mut count = count.borrow(key).borrow_mut(); agb::println!("Hello, world, frame = {}", *count); *count += 1; }, diff --git a/agb/examples/wave.rs b/agb/examples/wave.rs index 7571eef..f89cd6d 100644 --- a/agb/examples/wave.rs +++ b/agb/examples/wave.rs @@ -31,8 +31,8 @@ fn main(mut gba: agb::Gba) -> ! { let back = Mutex::new(RefCell::new(BackCosines { cosines, row: 0 })); - let _a = agb::interrupt::add_interrupt_handler(Interrupt::HBlank, |key: &CriticalSection| { - let mut backc = back.borrow(*key).borrow_mut(); + let _a = agb::interrupt::add_interrupt_handler(Interrupt::HBlank, |key: CriticalSection| { + let mut backc = back.borrow(key).borrow_mut(); let deflection = backc.cosines[backc.row % 32]; unsafe { ((0x0400_0010) as *mut u16).write_volatile(deflection) } backc.row += 1; @@ -43,7 +43,7 @@ fn main(mut gba: agb::Gba) -> ! { loop { vblank.wait_for_vblank(); free(|key| { - let mut backc = back.borrow(*key).borrow_mut(); + let mut backc = back.borrow(key).borrow_mut(); backc.row = 0; time += 1; for (r, a) in backc.cosines.iter_mut().enumerate() { diff --git a/agb/src/agb_alloc/block_allocator.rs b/agb/src/agb_alloc/block_allocator.rs index 99248e6..a3e51cb 100644 --- a/agb/src/agb_alloc/block_allocator.rs +++ b/agb/src/agb_alloc/block_allocator.rs @@ -61,7 +61,7 @@ impl BlockAllocator { #[cfg(test)] pub unsafe fn number_of_blocks(&self) -> u32 { free(|key| { - let mut state = self.state.borrow(*key).borrow_mut(); + let mut state = self.state.borrow(key).borrow_mut(); let mut count = 0; @@ -76,7 +76,7 @@ impl BlockAllocator { } /// Requests a brand new block from the inner bump allocator - fn new_block(&self, layout: Layout, cs: &CriticalSection) -> Option> { + fn new_block(&self, layout: Layout, cs: CriticalSection) -> Option> { let overall_layout = Block::either_layout(layout); self.inner_allocator.alloc_critical(overall_layout, cs) } @@ -84,7 +84,7 @@ impl BlockAllocator { /// Merges blocks together to create a normalised list unsafe fn normalise(&self) { free(|key| { - let mut state = self.state.borrow(*key).borrow_mut(); + let mut state = self.state.borrow(key).borrow_mut(); let mut list_ptr = &mut state.first_free_block; @@ -121,7 +121,7 @@ impl BlockAllocator { .unwrap(); free(|key| { - let mut state = self.state.borrow(*key).borrow_mut(); + let mut state = self.state.borrow(key).borrow_mut(); let mut current_block = state.first_free_block; let mut list_ptr = &mut state.first_free_block; // This iterates the free list until it either finds a block that @@ -164,7 +164,7 @@ impl BlockAllocator { pub unsafe fn dealloc_no_normalise(&self, ptr: *mut u8, layout: Layout) { let new_layout = Block::either_layout(layout).pad_to_align(); free(|key| { - let mut state = self.state.borrow(*key).borrow_mut(); + let mut state = self.state.borrow(key).borrow_mut(); // note that this is a reference to a pointer let mut list_ptr = &mut state.first_free_block; diff --git a/agb/src/agb_alloc/bump_allocator.rs b/agb/src/agb_alloc/bump_allocator.rs index daafb3c..717ba45 100644 --- a/agb/src/agb_alloc/bump_allocator.rs +++ b/agb/src/agb_alloc/bump_allocator.rs @@ -26,13 +26,13 @@ impl BumpAllocator { } impl BumpAllocator { - pub fn alloc_critical(&self, layout: Layout, cs: &CriticalSection) -> Option> { - let mut current_ptr = self.current_ptr.borrow(*cs).borrow_mut(); + pub fn alloc_critical(&self, layout: Layout, cs: CriticalSection) -> Option> { + let mut current_ptr = self.current_ptr.borrow(cs).borrow_mut(); let ptr = if let Some(c) = *current_ptr { c.as_ptr() as usize } else { - (self.start_end.borrow(*cs).start)() + (self.start_end.borrow(cs).start)() }; let alignment_bitmask = layout.align() - 1; @@ -43,7 +43,7 @@ impl BumpAllocator { let resulting_ptr = ptr + amount_to_add; let new_current_ptr = resulting_ptr + layout.size(); - if new_current_ptr as usize >= (self.start_end.borrow(*cs).end)() { + if new_current_ptr as usize >= (self.start_end.borrow(cs).end)() { return None; } diff --git a/agb/src/display/object.rs b/agb/src/display/object.rs index 4982b70..5a62a66 100644 --- a/agb/src/display/object.rs +++ b/agb/src/display/object.rs @@ -56,7 +56,7 @@ impl ObjectControllerRef { #[cfg(debug_assertions)] { let a = crate::interrupt::free(|c| { - let mut b = OBJECT_REFS_CURRENT.borrow(*c).borrow_mut(); + let mut b = OBJECT_REFS_CURRENT.borrow(c).borrow_mut(); let a = *b; *b += 1; a @@ -76,7 +76,7 @@ impl ObjectControllerRef { impl Drop for ObjectControllerRef { fn drop(&mut self) { crate::interrupt::free(|c| { - let mut b = OBJECT_REFS_CURRENT.borrow(*c).borrow_mut(); + let mut b = OBJECT_REFS_CURRENT.borrow(c).borrow_mut(); *b -= 1; }) } diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 19af7ba..db93655 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -59,7 +59,7 @@ type HashType = u32; /// A hash map implemented very simply using robin hood hashing. /// -/// HashMap uses FxHasher internally, which is a very fast hashing algorithm used +/// `HashMap` uses `FxHasher` internally, which is a very fast hashing algorithm used /// by rustc and firefox in non-adversarial places. It is incredibly fast, and good /// enough for most cases. /// @@ -76,7 +76,7 @@ type HashType = u32; /// aborts, memory leaks and non-termination. /// /// The API surface provided is incredibly similar to the -/// [std::collections::HashMap](https://doc.rust-lang.org/std/collections/struct.HashMap.html) +/// [`std::collections::HashMap`](https://doc.rust-lang.org/std/collections/struct.HashMap.html) /// implementation with fewer guarantees, and better optimised for the GameBoy Advance. /// /// [`Eq`]: https://doc.rust-lang.org/core/cmp/trait.Eq.html diff --git a/agb/src/interrupt.rs b/agb/src/interrupt.rs index 1489f08..2c80dcb 100644 --- a/agb/src/interrupt.rs +++ b/agb/src/interrupt.rs @@ -157,17 +157,17 @@ extern "C" fn __RUST_INTERRUPT_HANDLER(interrupt: u16) -> u16 { struct InterruptInner { next: Cell<*const InterruptInner>, root: *const InterruptRoot, - closure: *const dyn Fn(&CriticalSection), + closure: *const dyn Fn(CriticalSection), _pin: PhantomPinned, } unsafe fn create_interrupt_inner( - c: impl Fn(&CriticalSection), + c: impl Fn(CriticalSection), root: *const InterruptRoot, ) -> Pin> { let c = Box::new(c); - let c: &dyn Fn(&CriticalSection) = Box::leak(c); - let c: &dyn Fn(&CriticalSection) = core::mem::transmute(c); + let c: &dyn Fn(CriticalSection) = Box::leak(c); + let c: &dyn Fn(CriticalSection) = core::mem::transmute(c); Box::pin(InterruptInner { next: Cell::new(core::ptr::null()), root, @@ -216,7 +216,7 @@ impl InterruptRoot { while !c.is_null() { let closure_ptr = unsafe { &*c }.closure; let closure_ref = unsafe { &*closure_ptr }; - closure_ref(unsafe { &CriticalSection::new() }); + closure_ref(unsafe { CriticalSection::new() }); c = unsafe { &*c }.next.get(); } } @@ -241,7 +241,7 @@ fn interrupt_to_root(interrupt: Interrupt) -> &'static InterruptRoot { /// ``` pub fn add_interrupt_handler<'a>( interrupt: Interrupt, - handler: impl Fn(&CriticalSection) + Send + Sync + 'a, + handler: impl Fn(CriticalSection) + Send + Sync + 'a, ) -> InterruptHandler<'a> { fn do_with_inner<'a>( interrupt: Interrupt, @@ -282,13 +282,13 @@ pub fn add_interrupt_handler<'a>( /// [`CriticalSection`]: bare_metal::CriticalSection pub fn free(f: F) -> R where - F: FnOnce(&CriticalSection) -> R, + F: FnOnce(CriticalSection) -> R, { let enabled = INTERRUPTS_ENABLED.get(); disable_interrupts(); - let r = f(unsafe { &CriticalSection::new() }); + let r = f(unsafe { CriticalSection::new() }); INTERRUPTS_ENABLED.set(enabled); r @@ -328,17 +328,17 @@ mod tests { { let counter = Mutex::new(RefCell::new(0)); let counter_2 = Mutex::new(RefCell::new(0)); - let _a = add_interrupt_handler(Interrupt::VBlank, |key: &CriticalSection| { - *counter.borrow(*key).borrow_mut() += 1 + let _a = add_interrupt_handler(Interrupt::VBlank, |key: CriticalSection| { + *counter.borrow(key).borrow_mut() += 1 }); - let _b = add_interrupt_handler(Interrupt::VBlank, |key: &CriticalSection| { - *counter_2.borrow(*key).borrow_mut() += 1 + let _b = add_interrupt_handler(Interrupt::VBlank, |key: CriticalSection| { + *counter_2.borrow(key).borrow_mut() += 1 }); let vblank = VBlank::get(); while free(|key| { - *counter.borrow(*key).borrow() < 100 || *counter_2.borrow(*key).borrow() < 100 + *counter.borrow(key).borrow() < 100 || *counter_2.borrow(key).borrow() < 100 }) { vblank.wait_for_vblank(); } @@ -375,7 +375,7 @@ pub fn profiler(timer: &mut crate::timer::Timer, period: u16) -> InterruptHandle timer.set_overflow_amount(period); timer.set_enabled(true); - add_interrupt_handler(timer.interrupt(), |_key: &CriticalSection| { + add_interrupt_handler(timer.interrupt(), |_key: CriticalSection| { crate::println!("{:#010x}", crate::program_counter_before_interrupt()); }) } diff --git a/agb/src/rng.rs b/agb/src/rng.rs index 8d73a81..69d8bdf 100644 --- a/agb/src/rng.rs +++ b/agb/src/rng.rs @@ -55,7 +55,7 @@ static GLOBAL_RNG: Mutex> = /// Using a global random number generator, provides the next random number pub fn gen() -> i32 { - free(|cs| GLOBAL_RNG.borrow(*cs).borrow_mut().gen()) + free(|cs| GLOBAL_RNG.borrow(cs).borrow_mut().gen()) } #[cfg(test)] diff --git a/agb/src/sound/mixer/sw_mixer.rs b/agb/src/sound/mixer/sw_mixer.rs index 629dd3e..05c619c 100644 --- a/agb/src/sound/mixer/sw_mixer.rs +++ b/agb/src/sound/mixer/sw_mixer.rs @@ -216,11 +216,11 @@ impl MixerBuffer { } fn should_calculate(&self) -> bool { - free(|cs| self.state.borrow(*cs).borrow().should_calculate()) + free(|cs| self.state.borrow(cs).borrow().should_calculate()) } - fn swap(&self, cs: &CriticalSection) { - let buffer = self.state.borrow(*cs).borrow_mut().playing_advanced(); + fn swap(&self, cs: CriticalSection) { + let buffer = self.state.borrow(cs).borrow_mut().playing_advanced(); let (left_buffer, right_buffer) = self.buffers[buffer] .0 @@ -282,7 +282,7 @@ impl MixerBuffer { channel.pos += playback_speed * constants::SOUND_BUFFER_SIZE; } - let write_buffer_index = free(|cs| self.state.borrow(*cs).borrow_mut().active_advanced()); + let write_buffer_index = free(|cs| self.state.borrow(cs).borrow_mut().active_advanced()); let write_buffer = &mut self.buffers[write_buffer_index].0; cpu_fast_fill_i8(write_buffer, 0); From 1b393cd612cca9ffe0d6c9b9a129a6ca11ea2999 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 11 Jun 2022 20:40:12 +0100 Subject: [PATCH 04/11] Fix new clippy lints in tiled --- .../display/tiled/infinite_scrolled_map.rs | 5 +-- agb/src/display/tiled/map.rs | 3 +- agb/src/display/tiled/mod.rs | 32 +++++++++---------- agb/src/display/tiled/vram_manager.rs | 17 +++++++--- 4 files changed, 33 insertions(+), 24 deletions(-) diff --git a/agb/src/display/tiled/infinite_scrolled_map.rs b/agb/src/display/tiled/infinite_scrolled_map.rs index 41dc67d..98c2d6e 100644 --- a/agb/src/display/tiled/infinite_scrolled_map.rs +++ b/agb/src/display/tiled/infinite_scrolled_map.rs @@ -24,6 +24,7 @@ pub enum PartialUpdateStatus { } impl<'a> InfiniteScrolledMap<'a> { + #[must_use] pub fn new( map: MapLoan<'a, RegularMap>, tile: Box) -> (&'a TileSet<'a>, TileSetting) + 'a>, @@ -190,8 +191,8 @@ impl<'a> InfiniteScrolledMap<'a> { let current_scroll = self.map.scroll_pos(); let new_scroll = ( - size.px_offset_x(current_scroll.x as i32 + difference.x), - size.px_offset_y(current_scroll.y as i32 + difference.y), + size.px_offset_x(i32::from(current_scroll.x) + difference.x), + size.px_offset_y(i32::from(current_scroll.y) + difference.y), ) .into(); diff --git a/agb/src/display/tiled/map.rs b/agb/src/display/tiled/map.rs index 34cdc77..fbe5b69 100644 --- a/agb/src/display/tiled/map.rs +++ b/agb/src/display/tiled/map.rs @@ -105,7 +105,7 @@ impl RegularMap { pub fn commit(&mut self, vram: &mut VRamManager) { let new_bg_control_value = (self.priority as u16) - | ((self.screenblock as u16) << 8) + | (u16::from(self.screenblock) << 8) | (self.size.size_flag() << 14); self.bg_control_register().set(new_bg_control_value); @@ -136,6 +136,7 @@ impl RegularMap { self.y_scroll = pos.y; } + #[must_use] pub fn scroll_pos(&self) -> Vector2D { (self.x_scroll, self.y_scroll).into() } diff --git a/agb/src/display/tiled/mod.rs b/agb/src/display/tiled/mod.rs index 1590779..37e2f44 100644 --- a/agb/src/display/tiled/mod.rs +++ b/agb/src/display/tiled/mod.rs @@ -18,25 +18,23 @@ pub enum RegularBackgroundSize { } impl RegularBackgroundSize { + #[must_use] pub fn width(&self) -> u32 { match self { - RegularBackgroundSize::Background32x32 => 32, - RegularBackgroundSize::Background64x32 => 64, - RegularBackgroundSize::Background32x64 => 32, - RegularBackgroundSize::Background64x64 => 64, + RegularBackgroundSize::Background32x64 | RegularBackgroundSize::Background32x32 => 32, + RegularBackgroundSize::Background64x64 | RegularBackgroundSize::Background64x32 => 64, } } + #[must_use] pub fn height(&self) -> u32 { match self { - RegularBackgroundSize::Background32x32 => 32, - RegularBackgroundSize::Background64x32 => 32, - RegularBackgroundSize::Background32x64 => 64, - RegularBackgroundSize::Background64x64 => 64, + RegularBackgroundSize::Background32x32 | RegularBackgroundSize::Background64x32 => 32, + RegularBackgroundSize::Background32x64 | RegularBackgroundSize::Background64x64 => 64, } } - pub(crate) fn size_flag(&self) -> u16 { + pub(crate) fn size_flag(self) -> u16 { match self { RegularBackgroundSize::Background32x32 => 0, RegularBackgroundSize::Background64x32 => 1, @@ -45,17 +43,17 @@ impl RegularBackgroundSize { } } - pub(crate) fn num_tiles(&self) -> usize { + pub(crate) fn num_tiles(self) -> usize { (self.width() * self.height()) as usize } - pub(crate) fn num_screen_blocks(&self) -> usize { + pub(crate) fn num_screen_blocks(self) -> usize { self.num_tiles() / (32 * 32) } // This is hilariously complicated due to how the GBA stores the background screenblocks. // See https://www.coranac.com/tonc/text/regbg.htm#sec-map for an explanation - pub(crate) fn gba_offset(&self, pos: Vector2D) -> usize { + pub(crate) fn gba_offset(self, pos: Vector2D) -> usize { let x_mod = pos.x & (self.width() as u16 - 1); let y_mod = pos.y & (self.height() as u16 - 1); @@ -66,19 +64,19 @@ impl RegularBackgroundSize { pos as usize } - pub(crate) fn tile_pos_x(&self, x: i32) -> u16 { + pub(crate) fn tile_pos_x(self, x: i32) -> u16 { ((x as u32) & (self.width() - 1)) as u16 } - pub(crate) fn tile_pos_y(&self, y: i32) -> u16 { + pub(crate) fn tile_pos_y(self, y: i32) -> u16 { ((y as u32) & (self.height() - 1)) as u16 } - pub(crate) fn px_offset_x(&self, x: i32) -> u16 { + pub(crate) fn px_offset_x(self, x: i32) -> u16 { ((x as u32) & (self.width() * 8 - 1)) as u16 } - pub(crate) fn px_offset_y(&self, y: i32) -> u16 { + pub(crate) fn px_offset_y(self, y: i32) -> u16 { ((y as u32) & (self.height() * 8 - 1)) as u16 } } @@ -101,6 +99,7 @@ impl Tile { pub struct TileSetting(u16); impl TileSetting { + #[must_use] pub const fn new(tile_id: u16, hflip: bool, vflip: bool, palette_id: u8) -> Self { Self( (tile_id & ((1 << 10) - 1)) @@ -110,6 +109,7 @@ impl TileSetting { ) } + #[must_use] pub const fn from_raw(raw: u16) -> Self { Self(raw) } diff --git a/agb/src/display/tiled/vram_manager.rs b/agb/src/display/tiled/vram_manager.rs index bedbe80..a2a2ce1 100644 --- a/agb/src/display/tiled/vram_manager.rs +++ b/agb/src/display/tiled/vram_manager.rs @@ -44,6 +44,7 @@ pub struct TileSet<'a> { } impl<'a> TileSet<'a> { + #[must_use] pub fn new(tiles: &'a [u8], format: TileFormat) -> Self { Self { tiles, format } } @@ -53,7 +54,7 @@ impl<'a> TileSet<'a> { } } -#[derive(Debug)] +#[derive(Debug, Clone, Copy)] pub struct TileIndex(u16); impl TileIndex { @@ -61,7 +62,7 @@ impl TileIndex { Self(index as u16) } - pub(crate) const fn index(&self) -> u16 { + pub(crate) const fn index(self) -> u16 { self.0 } } @@ -128,8 +129,9 @@ pub struct DynamicTile<'a> { } impl DynamicTile<'_> { + #[must_use] pub fn fill_with(self, colour_index: u8) -> Self { - let colour_index = colour_index as u32; + let colour_index = u32::from(colour_index); let mut value = 0; for i in 0..8 { @@ -142,6 +144,7 @@ impl DynamicTile<'_> { } impl DynamicTile<'_> { + #[must_use] pub fn tile_set(&self) -> TileSet<'_> { let tiles = unsafe { slice::from_raw_parts_mut( @@ -153,6 +156,7 @@ impl DynamicTile<'_> { TileSet::new(tiles, TileFormat::FourBpp) } + #[must_use] pub fn tile_index(&self) -> u16 { let difference = self.tile_data.as_ptr() as usize - TILE_RAM_START; (difference / (8 * 8 / 2)) as u16 @@ -188,6 +192,7 @@ impl VRamManager { TileReference(NonNull::new(ptr as *mut _).unwrap()) } + #[must_use] pub fn new_dynamic_tile<'a>(&mut self) -> DynamicTile<'a> { let tile_format = TileFormat::FourBpp; let new_reference: NonNull = @@ -227,6 +232,8 @@ impl VRamManager { } } + // This needs to take ownership of the dynamic tile because it will no longer be valid after this call + #[allow(clippy::needless_pass_by_value)] pub fn remove_dynamic_tile(&mut self, dynamic_tile: DynamicTile<'_>) { let pointer = NonNull::new(dynamic_tile.tile_data.as_mut_ptr() as *mut _).unwrap(); let tile_reference = TileReference(pointer); @@ -336,7 +343,7 @@ impl VRamManager { tile_slice.as_ptr() as *const u16, target_location, tile_size_in_half_words, - ) + ); }; } @@ -356,7 +363,7 @@ impl VRamManager { /// Copies palettes to the background palettes without any checks. pub fn set_background_palettes(&mut self, palettes: &[palette16::Palette16]) { for (palette_index, entry) in palettes.iter().enumerate() { - self.set_background_palette(palette_index as u8, entry) + self.set_background_palette(palette_index as u8, entry); } } } From 062e8c888152cb8d9c85aad4afb5d6db509563da Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 11 Jun 2022 20:48:18 +0100 Subject: [PATCH 05/11] Fix some more linter errors --- agb/src/display/bitmap3.rs | 2 +- agb/src/display/bitmap4.rs | 7 +++--- agb/src/display/font.rs | 23 +++++++++-------- agb/src/display/object.rs | 48 ++++++++++++++++++++++++------------ agb/src/display/tile_data.rs | 1 + agb/src/lib.rs | 1 - 6 files changed, 51 insertions(+), 31 deletions(-) diff --git a/agb/src/display/bitmap3.rs b/agb/src/display/bitmap3.rs index 8a281a8..01a69e1 100644 --- a/agb/src/display/bitmap3.rs +++ b/agb/src/display/bitmap3.rs @@ -24,6 +24,6 @@ impl Bitmap3 { pub fn draw_point(&mut self, x: i32, y: i32, colour: u16) { let x = x.try_into().unwrap(); let y = y.try_into().unwrap(); - BITMAP_MODE_3.set(x, y, colour) + BITMAP_MODE_3.set(x, y, colour); } } diff --git a/agb/src/display/bitmap4.rs b/agb/src/display/bitmap4.rs index fdaa0f6..cd2debc 100644 --- a/agb/src/display/bitmap4.rs +++ b/agb/src/display/bitmap4.rs @@ -18,6 +18,7 @@ const BITMAP_PAGE_BACK_MODE_4: MemoryMapped2DArray< const PALETTE_BACKGROUND: MemoryMapped1DArray = unsafe { MemoryMapped1DArray::new(0x0500_0000) }; +#[derive(Clone, Copy)] pub enum Page { Front = 0, Back = 1, @@ -47,9 +48,9 @@ impl Bitmap4 { let c = addr.get(x_in_screen, y_in_screen); if x & 0b1 != 0 { - addr.set(x_in_screen, y_in_screen, c | (colour as u16) << 8); + addr.set(x_in_screen, y_in_screen, c | u16::from(colour) << 8); } else { - addr.set(x_in_screen, y_in_screen, c | colour as u16); + addr.set(x_in_screen, y_in_screen, c | u16::from(colour)); } } @@ -66,7 +67,7 @@ impl Bitmap4 { Page::Back }; - self.draw_point_page(x, y, colour, page) + self.draw_point_page(x, y, colour, page); } /// Sets the colour of colour index in the background palette. diff --git a/agb/src/display/font.rs b/agb/src/display/font.rs index 8bbe9ac..cb589ec 100644 --- a/agb/src/display/font.rs +++ b/agb/src/display/font.rs @@ -15,6 +15,7 @@ pub struct FontLetter { } impl FontLetter { + #[must_use] pub const fn new( width: u8, height: u8, @@ -41,6 +42,7 @@ pub struct Font { } impl Font { + #[must_use] pub const fn new(letters: &'static [FontLetter], line_height: i32, ascent: i32) -> Self { Self { letters, @@ -102,7 +104,7 @@ impl<'a> Write for TextRenderer<'a> { self.render_letter(letter); - self.current_x_pos += letter.advance_width as i32; + self.current_x_pos += i32::from(letter.advance_width); } Ok(()) @@ -119,9 +121,10 @@ impl<'a> TextRenderer<'a> { let foreground_colour = self.foreground_colour; let background_colour = self.background_colour; - let x_start = (self.current_x_pos + letter.xmin as i32).max(0); - let y_start = - self.current_y_pos + self.font.ascent - letter.height as i32 - letter.ymin as i32; + let x_start = (self.current_x_pos + i32::from(letter.xmin)).max(0); + let y_start = self.current_y_pos + self.font.ascent + - i32::from(letter.height) + - i32::from(letter.ymin); let x_tile_start = x_start / 8; let y_tile_start = y_start / 8; @@ -129,20 +132,20 @@ impl<'a> TextRenderer<'a> { let letter_offset_x = x_start.rem_euclid(8); let letter_offset_y = y_start.rem_euclid(8); - let x_tiles = div_ceil(letter.width as i32 + letter_offset_x, 8); - let y_tiles = div_ceil(letter.height as i32 + letter_offset_y, 8); + let x_tiles = div_ceil(i32::from(letter.width) + letter_offset_x, 8); + let y_tiles = div_ceil(i32::from(letter.height) + letter_offset_y, 8); for letter_y_tile in 0..(y_tiles + 1) { let letter_y_start = 0.max(letter_offset_y - 8 * letter_y_tile) + 8 * letter_y_tile; let letter_y_end = - (letter_offset_y + letter.height as i32).min((letter_y_tile + 1) * 8); + (letter_offset_y + i32::from(letter.height)).min((letter_y_tile + 1) * 8); let tile_y = y_tile_start + letter_y_tile; for letter_x_tile in 0..(x_tiles + 1) { let letter_x_start = 0.max(letter_offset_x - 8 * letter_x_tile) + 8 * letter_x_tile; let letter_x_end = - (letter_offset_x + letter.width as i32).min((letter_x_tile + 1) * 8); + (letter_offset_x + i32::from(letter.width)).min((letter_x_tile + 1) * 8); let tile_x = x_tile_start + letter_x_tile; @@ -154,13 +157,13 @@ impl<'a> TextRenderer<'a> { for letter_x in letter_x_start..letter_x_end { let x = letter_x - letter_offset_x; - let pos = x + y * letter.width as i32; + let pos = x + y * i32::from(letter.width); let px_line = letter.data[(pos / 8) as usize]; let px = (px_line >> (pos & 7)) & 1; if px != 0 { masks[(letter_y & 7) as usize] |= - (foreground_colour as u32) << ((letter_x & 7) * 4); + u32::from(foreground_colour) << ((letter_x & 7) * 4); zero = false; } } diff --git a/agb/src/display/object.rs b/agb/src/display/object.rs index 5a62a66..e714cd5 100644 --- a/agb/src/display/object.rs +++ b/agb/src/display/object.rs @@ -29,7 +29,7 @@ unsafe fn init_object_controller() { } unsafe fn uninit_object_controller() { - OBJECT_CONTROLLER.assume_init_drop() + OBJECT_CONTROLLER.assume_init_drop(); } struct ObjectControllerRef {} @@ -78,10 +78,12 @@ impl Drop for ObjectControllerRef { crate::interrupt::free(|c| { let mut b = OBJECT_REFS_CURRENT.borrow(c).borrow_mut(); *b -= 1; - }) + }); } } +// Required for safety reasons +#[allow(clippy::trivially_copy_pass_by_ref)] unsafe fn get_object_controller(_r: &ObjectControllerReference) -> ObjectControllerRef { ObjectControllerRef::new() } @@ -153,12 +155,15 @@ pub struct Graphics { } impl Graphics { + #[must_use] pub const fn new(sprites: &'static [Sprite], tag_map: &'static TagMap) -> Self { Self { sprites, tag_map } } + #[must_use] pub const fn tags(&self) -> &TagMap { self.tag_map } + #[must_use] pub const fn sprites(&self) -> &[Sprite] { self.sprites } @@ -184,9 +189,11 @@ const fn const_byte_compare(a: &[u8], b: &[u8]) -> bool { } impl TagMap { + #[must_use] pub const fn new(tags: &'static [(&'static str, Tag)]) -> TagMap { Self { tags } } + #[must_use] pub const fn try_get(&'static self, tag: &str) -> Option<&'static Tag> { let mut i = 0; while i < self.tags.len() { @@ -200,6 +207,7 @@ impl TagMap { None } + #[must_use] pub const fn get(&'static self, tag: &str) -> &'static Tag { let t = self.try_get(tag); match t { @@ -237,15 +245,18 @@ pub struct Tag { } impl Tag { + #[must_use] pub fn sprites(&self) -> &'static [Sprite] { unsafe { slice::from_raw_parts(self.sprites, self.len) } } + #[must_use] pub fn sprite(&self, idx: usize) -> &'static Sprite { &self.sprites()[idx] } #[inline] + #[must_use] pub fn animation_sprite(&self, idx: usize) -> &'static Sprite { let len_sub_1 = self.len - 1; match self.direction { @@ -259,6 +270,7 @@ impl Tag { } #[doc(hidden)] + #[must_use] pub const fn new(sprites: &'static [Sprite], from: usize, to: usize, direction: usize) -> Self { assert!(from <= to); assert!(to < sprites.len()); @@ -291,6 +303,7 @@ impl Size { (self as u8 >> 2, self as u8 & 0b11) } + #[must_use] pub const fn from_width_height(width: usize, height: usize) -> Self { match (width, height) { (8, 8) => Size::S8x8, @@ -309,6 +322,7 @@ impl Size { } } + #[must_use] pub const fn to_width_height(self) -> (usize, usize) { match self { Size::S8x8 => (8, 8), @@ -353,10 +367,10 @@ impl Storage { count: 1, } } - fn as_palette_ptr(&self) -> *mut u8 { + fn as_palette_ptr(self) -> *mut u8 { (self.location as usize * Palette16::layout().size() + PALETTE_SPRITE) as *mut u8 } - fn as_sprite_ptr(&self) -> *mut u8 { + fn as_sprite_ptr(self) -> *mut u8 { (self.location as usize * BYTES_PER_TILE_4BPP + TILE_SPRITE) as *mut u8 } } @@ -427,7 +441,7 @@ impl Drop for Loan<'_> { s.shadow_oam[self.index as usize] .as_mut() .unwrap_unchecked() - .destroy = true + .destroy = true; }; } } @@ -461,12 +475,8 @@ impl ObjectControllerStatic { fn update_z_ordering(&mut self) { let shadow_oam = &self.shadow_oam; - self.z_order.sort_by_key(|&a| { - shadow_oam[a as usize] - .as_ref() - .map(|s| s.z) - .unwrap_or(i32::MAX) - }); + self.z_order + .sort_by_key(|&a| shadow_oam[a as usize].as_ref().map_or(i32::MAX, |s| s.z)); } } @@ -498,7 +508,7 @@ impl ObjectController { unsafe { (OBJECT_ATTRIBUTE_MEMORY as *mut u16) .add((i as usize) * 4) - .write_volatile(HIDDEN_VALUE) + .write_volatile(HIDDEN_VALUE); } let a = unsafe { s.shadow_oam[z as usize].take().unwrap_unchecked() }; @@ -515,7 +525,7 @@ impl ObjectController { unsafe { (OBJECT_ATTRIBUTE_MEMORY as *mut u16) .add(i * 4) - .write_volatile(HIDDEN_VALUE) + .write_volatile(HIDDEN_VALUE); } } } @@ -530,7 +540,7 @@ impl ObjectController { unsafe { (OBJECT_ATTRIBUTE_MEMORY as *mut u16) .add(i * 4) - .write_volatile(HIDDEN_VALUE) + .write_volatile(HIDDEN_VALUE); } } @@ -540,10 +550,12 @@ impl ObjectController { } } + #[must_use] pub fn object<'a>(&'a self, sprite: SpriteBorrow<'a>) -> Object<'a> { self.try_get_object(sprite).expect("No object available") } + #[must_use] pub fn try_get_object<'a>(&'a self, sprite: SpriteBorrow<'a>) -> Option> { let mut s = unsafe { get_object_controller(&self.phantom) }; @@ -578,11 +590,13 @@ impl ObjectController { Some(Object { loan }) } + #[must_use] pub fn sprite(&self, sprite: &'static Sprite) -> SpriteBorrow { self.try_get_sprite(sprite) .expect("No slot for sprite available") } + #[must_use] pub fn try_get_sprite(&self, sprite: &'static Sprite) -> Option { let s = unsafe { get_object_controller(&self.phantom) }; unsafe { @@ -721,6 +735,7 @@ impl Sprite { fn layout(&self) -> Layout { Layout::from_size_align(self.size.number_of_tiles() * BYTES_PER_TILE_4BPP, 8).unwrap() } + #[must_use] pub const fn new(palette: &'static Palette16, data: &'static [u8], size: Size) -> Self { Self { palette, @@ -728,6 +743,7 @@ impl Sprite { size, } } + #[must_use] pub const fn size(&self) -> Size { self.size } @@ -823,7 +839,7 @@ impl SpriteControllerInner { } } - self.return_palette(sprite.palette) + self.return_palette(sprite.palette); } fn return_palette(&mut self, palette: &'static Palette16) { @@ -843,7 +859,7 @@ impl SpriteControllerInner { impl<'a> Drop for SpriteBorrow<'a> { fn drop(&mut self) { let mut s = unsafe { get_object_controller(&self.phantom) }; - s.sprite_controller.return_sprite(self.id.sprite()) + s.sprite_controller.return_sprite(self.id.sprite()); } } diff --git a/agb/src/display/tile_data.rs b/agb/src/display/tile_data.rs index 4412369..364b57c 100644 --- a/agb/src/display/tile_data.rs +++ b/agb/src/display/tile_data.rs @@ -7,6 +7,7 @@ pub struct TileData { } impl TileData { + #[must_use] pub const fn new( palettes: &'static [Palette16], tiles: &'static [u8], diff --git a/agb/src/lib.rs b/agb/src/lib.rs index 87854be..19f85d2 100644 --- a/agb/src/lib.rs +++ b/agb/src/lib.rs @@ -9,7 +9,6 @@ #![warn(clippy::must_use_candidate)] #![warn(clippy::trivially_copy_pass_by_ref)] #![warn(clippy::semicolon_if_nothing_returned)] -#![warn(clippy::match_same_arms)] #![warn(clippy::cast_lossless)] #![warn(clippy::map_unwrap_or)] #![warn(clippy::needless_pass_by_value)] From bf9f29897247678f66ff706206991fa318e34aa6 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 11 Jun 2022 20:51:41 +0100 Subject: [PATCH 06/11] Fix some sound related lints --- agb/src/display/palette16.rs | 2 ++ agb/src/sound/dmg.rs | 36 +++++++++++++++++++++--------------- agb/src/sound/mixer/mod.rs | 4 +++- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/agb/src/display/palette16.rs b/agb/src/display/palette16.rs index 55181b9..6a4441b 100644 --- a/agb/src/display/palette16.rs +++ b/agb/src/display/palette16.rs @@ -5,6 +5,7 @@ pub struct Palette16 { } impl Palette16 { + #[must_use] pub const fn new(colours: [u16; 16]) -> Self { Palette16 { colours } } @@ -16,6 +17,7 @@ impl Palette16 { self.colours[index] = colour; } + #[must_use] pub fn colour(&self, index: usize) -> u16 { self.colours[index] } diff --git a/agb/src/sound/dmg.rs b/agb/src/sound/dmg.rs index 4a91aa6..206948e 100644 --- a/agb/src/sound/dmg.rs +++ b/agb/src/sound/dmg.rs @@ -22,14 +22,17 @@ impl Sound { Sound {} } + #[must_use] pub fn channel1(&self) -> Channel1 { Channel1 {} } + #[must_use] pub fn channel2(&self) -> Channel2 { Channel2 {} } + #[must_use] pub fn noise(&self) -> Noise { Noise {} } @@ -56,10 +59,10 @@ impl Channel1 { duty_cycle: DutyCycle, ) { CHANNEL_1_SWEEP.set(sweep_settings.as_bits()); - let length_bits = length.unwrap_or(0) as u16; + let length_bits = u16::from(length.unwrap_or(0)); assert!(length_bits < 64, "Length must be less than 64"); - let length_flag: u16 = length.map(|_| 1 << 14).unwrap_or(0); + let length_flag: u16 = length.map_or(0, |_| 1 << 14); let initial: u16 = 1 << 15; assert!(frequency < 2048, "Frequency must be less than 2048"); @@ -81,10 +84,10 @@ impl Channel2 { envelope_settings: &EnvelopeSettings, duty_cycle: DutyCycle, ) { - let length_bits = length.unwrap_or(0) as u16; + let length_bits = u16::from(length.unwrap_or(0)); assert!(length_bits < 64, "Length must be less than 64"); - let length_flag: u16 = length.map(|_| 1 << 14).unwrap_or(0); + let length_flag: u16 = length.map_or(0, |_| 1 << 14); let initial: u16 = 1 << 15; assert!(frequency < 2048, "Frequency must be less than 2048"); @@ -107,7 +110,7 @@ impl Noise { counter_step_width_15: bool, shift_clock_frequency: u8, ) { - let length_bits = length.unwrap_or(0) as u16; + let length_bits = u16::from(length.unwrap_or(0)); assert!(length_bits < 64, "length must be less than 16"); assert!( @@ -119,19 +122,19 @@ impl Noise { "frequency clock divider must be less than 16" ); - let length_flag: u16 = length.map(|_| 1 << 14).unwrap_or(0); + let length_flag: u16 = length.map_or(0, |_| 1 << 14); let initial: u16 = 1 << 15; let counter_step_bit = if counter_step_width_15 { 0 } else { 1 << 3 }; CHANNEL_4_LENGTH_ENVELOPE.set(length_bits | envelope_setting.as_bits()); CHANNEL_4_FREQUENCY_CONTROL.set( - (frequency_divider as u16) + u16::from(frequency_divider) | counter_step_bit - | ((shift_clock_frequency as u16) << 4) + | (u16::from(shift_clock_frequency) << 4) | length_flag | initial, - ) + ); } } @@ -156,6 +159,7 @@ pub struct SweepSettings { } impl SweepSettings { + #[must_use] pub fn new( number_of_sweep_shifts: u8, sound_direction: SoundDirection, @@ -175,9 +179,9 @@ impl SweepSettings { } fn as_bits(&self) -> u16 { - ((self.number_of_sweep_shifts as u16) & 0b111) + (u16::from(self.number_of_sweep_shifts) & 0b111) | ((1 - self.sound_direction.as_bits()) << 3) // sweep works backwards - | ((self.sweep_time as u16) & 0b111) << 4 + | (u16::from(self.sweep_time) & 0b111) << 4 } } @@ -194,6 +198,7 @@ pub struct EnvelopeSettings { } impl EnvelopeSettings { + #[must_use] pub fn new(step_time: u8, direction: SoundDirection, initial_volume: u8) -> Self { assert!(step_time < 8, "Step time must be less than 8"); assert!(initial_volume < 16, "Initial volume must be less that 16"); @@ -205,9 +210,9 @@ impl EnvelopeSettings { } fn as_bits(&self) -> u16 { - (self.step_time as u16) << 8 + u16::from(self.step_time) << 8 | (self.direction.as_bits() << 11) - | ((self.initial_volume as u16) << 12) + | (u16::from(self.initial_volume) << 12) } } @@ -217,6 +222,7 @@ impl Default for EnvelopeSettings { } } +#[derive(Copy, Clone)] pub enum DutyCycle { OneEighth, OneQuarter, @@ -225,10 +231,10 @@ pub enum DutyCycle { } impl DutyCycle { - fn as_bits(&self) -> u16 { + fn as_bits(self) -> u16 { use DutyCycle::*; - match &self { + match self { OneEighth => 0, OneQuarter => 1, Half => 2, diff --git a/agb/src/sound/mixer/mod.rs b/agb/src/sound/mixer/mod.rs index 0022707..134fb60 100644 --- a/agb/src/sound/mixer/mod.rs +++ b/agb/src/sound/mixer/mod.rs @@ -43,6 +43,7 @@ pub struct SoundChannel { impl SoundChannel { #[inline(always)] + #[must_use] pub fn new(data: &'static [u8]) -> Self { SoundChannel { data, @@ -58,6 +59,7 @@ impl SoundChannel { } #[inline(always)] + #[must_use] pub fn new_high_priority(data: &'static [u8]) -> Self { SoundChannel { data, @@ -110,6 +112,6 @@ impl SoundChannel { #[inline(always)] pub fn stop(&mut self) { - self.is_done = true + self.is_done = true; } } From ba35b85c81ab71fd83caed830eaae094fff2ae53 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 11 Jun 2022 20:56:05 +0100 Subject: [PATCH 07/11] Final clippy lint fixes --- agb/src/bitarray.rs | 4 ++-- agb/src/hash_map.rs | 18 +++++++++--------- agb/src/input.rs | 22 +++++++++++++++------- agb/src/interrupt.rs | 6 ++++-- agb/src/lib.rs | 3 ++- agb/src/mgba.rs | 3 ++- agb/src/rng.rs | 3 +++ agb/src/syscall.rs | 4 ++++ agb/src/timer.rs | 6 ++++-- 9 files changed, 45 insertions(+), 24 deletions(-) diff --git a/agb/src/bitarray.rs b/agb/src/bitarray.rs index f953ac1..9086e92 100644 --- a/agb/src/bitarray.rs +++ b/agb/src/bitarray.rs @@ -17,10 +17,10 @@ impl Bitarray { } pub fn set(&mut self, index: usize, value: bool) { - let value = value as u32; + let value = u32::from(value); let mask = 1 << (index % 32); let value_mask = value << (index % 32); - self.a[index / 32] = self.a[index / 32] & !mask | value_mask + self.a[index / 32] = self.a[index / 32] & !mask | value_mask; } pub fn first_zero(&self) -> Option { diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index db93655..36b0268 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -89,11 +89,13 @@ pub struct HashMap { impl HashMap { /// Creates a `HashMap` + #[must_use] pub fn new() -> Self { Self::with_size(16) } /// Creates an empty `HashMap` with specified internal size. The size must be a power of 2 + #[must_use] pub fn with_size(size: usize) -> Self { Self { nodes: NodeStorage::with_size(size), @@ -103,6 +105,7 @@ impl HashMap { /// Creates an empty `HashMap` which can hold at least `capacity` elements before resizing. The actual /// internal size may be larger as it must be a power of 2 + #[must_use] pub fn with_capacity(capacity: usize) -> Self { for i in 0..32 { let attempted_size = 1usize << i; @@ -118,11 +121,13 @@ impl HashMap { } /// Returns the number of elements in the map + #[must_use] pub fn len(&self) -> usize { self.nodes.len() } /// Returns the number of elements the map can hold + #[must_use] pub fn capacity(&self) -> usize { self.nodes.capacity() } @@ -149,21 +154,16 @@ impl HashMap { /// An iterator visiting all key-value pairs in an arbitrary order pub fn iter(&self) -> impl Iterator { - self.nodes - .nodes - .iter() - .filter_map(|node| node.key_value_ref()) + self.nodes.nodes.iter().filter_map(Node::key_value_ref) } /// An iterator visiting all key-value pairs in an arbitrary order, with mutable references to the values pub fn iter_mut(&mut self) -> impl Iterator { - self.nodes - .nodes - .iter_mut() - .filter_map(|node| node.key_value_mut()) + self.nodes.nodes.iter_mut().filter_map(Node::key_value_mut) } /// Returns `true` if the map contains no elements + #[must_use] pub fn is_empty(&self) -> bool { self.len() == 0 } @@ -1179,7 +1179,7 @@ mod test { fn test_entry(_gba: &mut Gba) { let xs = [(1, 10), (2, 20), (3, 30), (4, 40), (5, 50), (6, 60)]; - let mut map: HashMap<_, _> = xs.iter().cloned().collect(); + let mut map: HashMap<_, _> = xs.iter().copied().collect(); // Existing key (insert) match map.entry(1) { diff --git a/agb/src/input.rs b/agb/src/input.rs index ce8cf2d..7c77fbd 100644 --- a/agb/src/input.rs +++ b/agb/src/input.rs @@ -10,8 +10,8 @@ pub enum Tri { impl From<(bool, bool)> for Tri { fn from(a: (bool, bool)) -> Tri { - let b1 = a.0 as i8; - let b2 = a.1 as i8; + let b1 = i8::from(a.0); + let b2 = i8::from(a.1); unsafe { core::mem::transmute(b2 - b1) } } } @@ -47,6 +47,7 @@ impl Default for ButtonController { } impl ButtonController { + #[must_use] pub fn new() -> Self { let pressed = !unsafe { BUTTON_INPUT.read_volatile() }; ButtonController { @@ -60,6 +61,7 @@ impl ButtonController { self.current = !unsafe { BUTTON_INPUT.read_volatile() }; } + #[must_use] pub fn x_tri(&self) -> Tri { let left = self.is_pressed(Button::LEFT); let right = self.is_pressed(Button::RIGHT); @@ -67,6 +69,7 @@ impl ButtonController { (left, right).into() } + #[must_use] pub fn y_tri(&self) -> Tri { let up = self.is_pressed(Button::UP); let down = self.is_pressed(Button::DOWN); @@ -74,25 +77,30 @@ impl ButtonController { (up, down).into() } + #[must_use] pub fn is_pressed(&self, keys: Button) -> bool { - let currently_pressed = self.current as u32; + let currently_pressed = u32::from(self.current); let keys = keys.bits(); (currently_pressed & keys) != 0 } + + #[must_use] pub fn is_released(&self, keys: Button) -> bool { !self.is_pressed(keys) } + #[must_use] pub fn is_just_pressed(&self, keys: Button) -> bool { - let current = self.current as u32; - let previous = self.previous as u32; + let current = u32::from(self.current); + let previous = u32::from(self.previous); let keys = keys.bits(); ((current & keys) != 0) && ((previous & keys) == 0) } + #[must_use] pub fn is_just_released(&self, keys: Button) -> bool { - let current = self.current as u32; - let previous = self.previous as u32; + let current = u32::from(self.current); + let previous = u32::from(self.previous); let keys = keys.bits(); ((current & keys) == 0) && ((previous & keys) != 0) } diff --git a/agb/src/interrupt.rs b/agb/src/interrupt.rs index 2c80dcb..3d08d21 100644 --- a/agb/src/interrupt.rs +++ b/agb/src/interrupt.rs @@ -179,6 +179,7 @@ unsafe fn create_interrupt_inner( impl Drop for InterruptInner { fn drop(&mut self) { inner_drop(unsafe { Pin::new_unchecked(self) }); + #[allow(clippy::needless_pass_by_value)] // needed for safety reasons fn inner_drop(this: Pin<&mut InterruptInner>) { // drop the closure allocation safely let _closure_box = @@ -300,6 +301,7 @@ pub struct VBlank {} impl VBlank { /// Handles setting up everything reqired to be able to use the wait for /// interrupt syscall. + #[must_use] pub fn get() -> Self { interrupt_to_root(Interrupt::VBlank).add(); VBlank {} @@ -329,10 +331,10 @@ mod tests { let counter = Mutex::new(RefCell::new(0)); let counter_2 = Mutex::new(RefCell::new(0)); let _a = add_interrupt_handler(Interrupt::VBlank, |key: CriticalSection| { - *counter.borrow(key).borrow_mut() += 1 + *counter.borrow(key).borrow_mut() += 1; }); let _b = add_interrupt_handler(Interrupt::VBlank, |key: CriticalSection| { - *counter_2.borrow(key).borrow_mut() += 1 + *counter_2.borrow(key).borrow_mut() += 1; }); let vblank = VBlank::get(); diff --git a/agb/src/lib.rs b/agb/src/lib.rs index 19f85d2..910c867 100644 --- a/agb/src/lib.rs +++ b/agb/src/lib.rs @@ -234,6 +234,7 @@ pub struct Gba { impl Gba { #[doc(hidden)] + #[must_use] pub unsafe fn new_in_entry() -> Self { GBASINGLE.take() } @@ -365,7 +366,7 @@ mod test { break; } vblank.wait_for_vblank(); - counter += 1 + counter += 1; } } diff --git a/agb/src/mgba.rs b/agb/src/mgba.rs index 02481fa..1849eed 100644 --- a/agb/src/mgba.rs +++ b/agb/src/mgba.rs @@ -29,7 +29,7 @@ fn is_running_in_mgba() -> bool { const NUMBER_OF_CYCLES: MemoryMapped = unsafe { MemoryMapped::new(0x04FF_F800) }; pub fn number_of_cycles_tagged(tag: u16) { - NUMBER_OF_CYCLES.set(tag) + NUMBER_OF_CYCLES.set(tag); } pub struct Mgba { @@ -37,6 +37,7 @@ pub struct Mgba { } impl Mgba { + #[must_use] pub fn new() -> Option { if is_running_in_mgba() { Some(Mgba { bytes_written: 0 }) diff --git a/agb/src/rng.rs b/agb/src/rng.rs index 69d8bdf..282efad 100644 --- a/agb/src/rng.rs +++ b/agb/src/rng.rs @@ -15,12 +15,14 @@ impl RandomNumberGenerator { /// Create a new random number generator with a fixed seed /// /// Note that this seed is guaranteed to be the same between minor releases. + #[must_use] pub const fn new() -> Self { Self::new_with_seed([1014776995, 476057059, 3301633994, 706340607]) } /// Produces a random number generator with the given initial state / seed. /// None of the values can be 0. + #[must_use] pub const fn new_with_seed(seed: [u32; 4]) -> Self { // this can't be in a loop because const assert!(seed[0] != 0, "seed must not be 0"); @@ -54,6 +56,7 @@ static GLOBAL_RNG: Mutex> = Mutex::new(RefCell::new(RandomNumberGenerator::new())); /// Using a global random number generator, provides the next random number +#[must_use] pub fn gen() -> i32 { free(|cs| GLOBAL_RNG.borrow(cs).borrow_mut().gen()) } diff --git a/agb/src/syscall.rs b/agb/src/syscall.rs index da94ee6..82e2464 100644 --- a/agb/src/syscall.rs +++ b/agb/src/syscall.rs @@ -54,6 +54,7 @@ pub fn wait_for_vblank() { } } +#[must_use] pub fn div(numerator: i32, denominator: i32) -> (i32, i32, i32) { let divide: i32; let modulo: i32; @@ -71,6 +72,7 @@ pub fn div(numerator: i32, denominator: i32) -> (i32, i32, i32) { (divide, modulo, abs_divide) } +#[must_use] pub fn sqrt(n: i32) -> i32 { let result: i32; unsafe { @@ -86,6 +88,7 @@ pub fn sqrt(n: i32) -> i32 { result } +#[must_use] pub fn arc_tan(n: i16) -> i16 { let result: i16; unsafe { @@ -101,6 +104,7 @@ pub fn arc_tan(n: i16) -> i16 { result } +#[must_use] pub fn arc_tan2(x: i16, y: i32) -> i16 { let result: i16; unsafe { diff --git a/agb/src/timer.rs b/agb/src/timer.rs index 6fb1a9d..5797fcd 100644 --- a/agb/src/timer.rs +++ b/agb/src/timer.rs @@ -21,7 +21,7 @@ pub enum Divider { } impl Divider { - fn as_bits(&self) -> u16 { + fn as_bits(self) -> u16 { use Divider::*; match self { @@ -68,6 +68,7 @@ impl Timer { self } + #[must_use] pub fn value(&self) -> u16 { self.data_register().get() } @@ -90,7 +91,7 @@ impl Timer { } pub fn set_interrupt(&mut self, interrupt: bool) -> &mut Self { - let bit = interrupt as u16; + let bit = u16::from(interrupt); self.control_register().set_bits(bit, 1, 6); self } @@ -107,6 +108,7 @@ impl Timer { self.timer_number as usize } + #[must_use] pub fn interrupt(&self) -> crate::interrupt::Interrupt { use crate::interrupt::Interrupt; match self.timer_number { From 9759555e424dc265b0ad8e6936b84032c91e9d33 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 11 Jun 2022 20:56:48 +0100 Subject: [PATCH 08/11] Remove cast_lossless because it isn't supported by bitfield --- agb/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/agb/src/lib.rs b/agb/src/lib.rs index 910c867..1339ed7 100644 --- a/agb/src/lib.rs +++ b/agb/src/lib.rs @@ -9,7 +9,6 @@ #![warn(clippy::must_use_candidate)] #![warn(clippy::trivially_copy_pass_by_ref)] #![warn(clippy::semicolon_if_nothing_returned)] -#![warn(clippy::cast_lossless)] #![warn(clippy::map_unwrap_or)] #![warn(clippy::needless_pass_by_value)] #![warn(clippy::redundant_closure_for_method_calls)] From bbad55af6551753aae95b777e0cf47c8eb5a7e6a Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 11 Jun 2022 20:57:40 +0100 Subject: [PATCH 09/11] Deny the stricter lints --- agb/src/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/agb/src/lib.rs b/agb/src/lib.rs index 1339ed7..1ac38a4 100644 --- a/agb/src/lib.rs +++ b/agb/src/lib.rs @@ -6,13 +6,13 @@ #![cfg_attr(test, reexport_test_harness_main = "test_main")] #![feature(alloc_error_handler)] #![warn(clippy::all)] -#![warn(clippy::must_use_candidate)] -#![warn(clippy::trivially_copy_pass_by_ref)] -#![warn(clippy::semicolon_if_nothing_returned)] -#![warn(clippy::map_unwrap_or)] -#![warn(clippy::needless_pass_by_value)] -#![warn(clippy::redundant_closure_for_method_calls)] -#![warn(clippy::cloned_instead_of_copied)] +#![deny(clippy::must_use_candidate)] +#![deny(clippy::trivially_copy_pass_by_ref)] +#![deny(clippy::semicolon_if_nothing_returned)] +#![deny(clippy::map_unwrap_or)] +#![deny(clippy::needless_pass_by_value)] +#![deny(clippy::redundant_closure_for_method_calls)] +#![deny(clippy::cloned_instead_of_copied)] //! # agb //! `agb` is a library for making games on the Game Boy Advance using the Rust From 8224827f9046cd2fa647e4a58f228c6d60e7940f Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 11 Jun 2022 21:02:24 +0100 Subject: [PATCH 10/11] Fix warning (and make it obvious we're not caring about the return value) --- examples/the-purple-night/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/the-purple-night/src/main.rs b/examples/the-purple-night/src/main.rs index 4dc79b5..07fc59c 100644 --- a/examples/the-purple-night/src/main.rs +++ b/examples/the-purple-night/src/main.rs @@ -2291,7 +2291,7 @@ fn game_with_level(gba: &mut agb::Gba) { } } - rng::gen(); // advance RNG to make it less predictable between runs + let _ = rng::gen(); // advance RNG to make it less predictable between runs }; game.clear(&mut vram); From bec1a17cfdce760ab13a34546a720a8798179f38 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 12 Jun 2022 13:57:16 +0100 Subject: [PATCH 11/11] This doesn't need the allow --- agb/src/display/object.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/agb/src/display/object.rs b/agb/src/display/object.rs index e714cd5..0b88c3e 100644 --- a/agb/src/display/object.rs +++ b/agb/src/display/object.rs @@ -82,9 +82,7 @@ impl Drop for ObjectControllerRef { } } -// Required for safety reasons -#[allow(clippy::trivially_copy_pass_by_ref)] -unsafe fn get_object_controller(_r: &ObjectControllerReference) -> ObjectControllerRef { +unsafe fn get_object_controller(_r: ObjectControllerReference) -> ObjectControllerRef { ObjectControllerRef::new() } @@ -435,7 +433,7 @@ struct Loan<'a> { impl Drop for Loan<'_> { fn drop(&mut self) { - let mut s = unsafe { get_object_controller(&self.phantom) }; + let mut s = unsafe { get_object_controller(self.phantom) }; unsafe { s.shadow_oam[self.index as usize] @@ -496,7 +494,7 @@ const HIDDEN_VALUE: u16 = 0b10 << 8; impl ObjectController { pub fn commit(&self) { - let mut s = unsafe { get_object_controller(&self.phantom) }; + let mut s = unsafe { get_object_controller(self.phantom) }; let s = &mut *s; @@ -557,7 +555,7 @@ impl ObjectController { #[must_use] pub fn try_get_object<'a>(&'a self, sprite: SpriteBorrow<'a>) -> Option> { - let mut s = unsafe { get_object_controller(&self.phantom) }; + let mut s = unsafe { get_object_controller(self.phantom) }; let mut attrs = Attributes::new(); @@ -598,7 +596,7 @@ impl ObjectController { #[must_use] pub fn try_get_sprite(&self, sprite: &'static Sprite) -> Option { - let s = unsafe { get_object_controller(&self.phantom) }; + let s = unsafe { get_object_controller(self.phantom) }; unsafe { s.very_unsafe_borrow() .sprite_controller @@ -610,7 +608,7 @@ impl ObjectController { impl<'a> Object<'a> { #[inline(always)] unsafe fn object_inner(&mut self) -> &mut ObjectInner { - let s = get_object_controller(&self.loan.phantom); + let s = get_object_controller(self.loan.phantom); s.very_unsafe_borrow().shadow_oam[self.loan.index as usize] .as_mut() .unwrap_unchecked() @@ -679,7 +677,7 @@ impl<'a> Object<'a> { let object_inner = unsafe { self.object_inner() }; object_inner.z = z; unsafe { - get_object_controller(&self.loan.phantom).update_z_ordering(); + get_object_controller(self.loan.phantom).update_z_ordering(); } self @@ -858,7 +856,7 @@ impl SpriteControllerInner { impl<'a> Drop for SpriteBorrow<'a> { fn drop(&mut self) { - let mut s = unsafe { get_object_controller(&self.phantom) }; + let mut s = unsafe { get_object_controller(self.phantom) }; s.sprite_controller.return_sprite(self.id.sprite()); } } @@ -883,7 +881,7 @@ impl<'a> SpriteBorrow<'a> { impl<'a> Clone for SpriteBorrow<'a> { fn clone(&self) -> Self { - let mut s = unsafe { get_object_controller(&self.phantom) }; + let mut s = unsafe { get_object_controller(self.phantom) }; self.clone(&mut s.sprite_controller) } }