From dc67d2f4a49f4c9166fdefa4319a0e6dfab823e8 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 19 May 2021 22:31:09 +0200 Subject: [PATCH 1/3] impl Unborrow for &'a mut T This plays nicer with user code that's generic over peripheral traits like `Instance` or `Pin`. --- embassy-extras/src/macros.rs | 16 ---------------- embassy-macros/src/lib.rs | 7 ------- embassy/src/util/mod.rs | 18 +++++++----------- 3 files changed, 7 insertions(+), 34 deletions(-) diff --git a/embassy-extras/src/macros.rs b/embassy-extras/src/macros.rs index fba752619..351938c42 100644 --- a/embassy-extras/src/macros.rs +++ b/embassy-extras/src/macros.rs @@ -24,14 +24,6 @@ macro_rules! peripherals { } } - $(#[$cfg])? - impl embassy::util::Unborrow for &mut $name { - type Target = $name; - #[inline] - unsafe fn unborrow(self) -> $name { - ::core::ptr::read(self) - } - } )* } @@ -95,14 +87,6 @@ macro_rules! impl_unborrow { self } } - - impl<'a> ::embassy::util::Unborrow for &'a mut $type { - type Target = $type; - #[inline] - unsafe fn unborrow(self) -> Self::Target { - unsafe { ::core::ptr::read(self) } - } - } }; } diff --git a/embassy-macros/src/lib.rs b/embassy-macros/src/lib.rs index c2f928b93..ac856bef6 100644 --- a/embassy-macros/src/lib.rs +++ b/embassy-macros/src/lib.rs @@ -216,13 +216,6 @@ pub fn interrupt_declare(item: TokenStream) -> TokenStream { self } } - - impl ::embassy::util::Unborrow for &mut #name_interrupt { - type Target = #name_interrupt; - unsafe fn unborrow(self) -> #name_interrupt { - ::core::ptr::read(self) - } - } }; result.into() } diff --git a/embassy/src/util/mod.rs b/embassy/src/util/mod.rs index 7de15d4ad..8057e120e 100644 --- a/embassy/src/util/mod.rs +++ b/embassy/src/util/mod.rs @@ -22,6 +22,13 @@ pub trait Unborrow { unsafe fn unborrow(self) -> Self::Target; } +impl<'a, T: Unborrow> Unborrow for &'a mut T { + type Target = T::Target; + unsafe fn unborrow(self) -> Self::Target { + T::unborrow(core::ptr::read(self)) + } +} + pub trait Steal { unsafe fn steal() -> Self; } @@ -40,17 +47,6 @@ macro_rules! impl_unborrow_tuples { } } - impl<'a, $($t),+> Unborrow for &'a mut($($t),+) - where - $( - $t: Unborrow - ),+ - { - type Target = ($($t),+); - unsafe fn unborrow(self) -> Self::Target { - ::core::ptr::read(self) - } - } }; } From 105c8504b61c77bedc4123d157f674f40abc3e0b Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 19 May 2021 23:21:31 +0200 Subject: [PATCH 2/3] Mark Unborrow as unsafe to implement --- embassy-extras/src/macros.rs | 6 +++--- embassy-macros/src/lib.rs | 2 +- embassy-nrf/src/gpio.rs | 8 ++++---- embassy-nrf/src/gpiote.rs | 4 ++-- embassy-nrf/src/ppi.rs | 8 ++++---- embassy-nrf/src/saadc.rs | 2 +- embassy-rp/src/gpio.rs | 4 ++-- embassy-stm32/src/exti.rs | 4 ++-- embassy-stm32/src/gpio.rs | 6 +++--- embassy/src/util/mod.rs | 30 +++++++++++++++--------------- 10 files changed, 37 insertions(+), 37 deletions(-) diff --git a/embassy-extras/src/macros.rs b/embassy-extras/src/macros.rs index 351938c42..771db40f6 100644 --- a/embassy-extras/src/macros.rs +++ b/embassy-extras/src/macros.rs @@ -16,7 +16,7 @@ macro_rules! peripherals { } $(#[$cfg])? - impl embassy::util::Unborrow for $name { + unsafe impl embassy::util::Unborrow for $name { type Target = $name; #[inline] unsafe fn unborrow(self) -> $name { @@ -78,9 +78,9 @@ macro_rules! unborrow { } #[macro_export] -macro_rules! impl_unborrow { +macro_rules! unsafe_impl_unborrow { ($type:ident) => { - impl ::embassy::util::Unborrow for $type { + unsafe impl ::embassy::util::Unborrow for $type { type Target = $type; #[inline] unsafe fn unborrow(self) -> Self::Target { diff --git a/embassy-macros/src/lib.rs b/embassy-macros/src/lib.rs index ac856bef6..d23526aa2 100644 --- a/embassy-macros/src/lib.rs +++ b/embassy-macros/src/lib.rs @@ -210,7 +210,7 @@ pub fn interrupt_declare(item: TokenStream) -> TokenStream { } } - impl ::embassy::util::Unborrow for #name_interrupt { + unsafe impl ::embassy::util::Unborrow for #name_interrupt { type Target = #name_interrupt; unsafe fn unborrow(self) -> #name_interrupt { self diff --git a/embassy-nrf/src/gpio.rs b/embassy-nrf/src/gpio.rs index 223247d60..3ae160ca8 100644 --- a/embassy-nrf/src/gpio.rs +++ b/embassy-nrf/src/gpio.rs @@ -5,7 +5,7 @@ use core::hint::unreachable_unchecked; use core::marker::PhantomData; use embassy::util::Unborrow; -use embassy_extras::{impl_unborrow, unborrow}; +use embassy_extras::{unborrow, unsafe_impl_unborrow}; use embedded_hal::digital::v2::{InputPin, OutputPin, StatefulOutputPin}; use gpio::pin_cnf::DRIVE_A; @@ -330,7 +330,7 @@ pub(crate) mod sealed { pub trait OptionalPin {} } -pub trait Pin: Unborrow + sealed::Pin + Sized { +pub trait Pin: Unborrow + sealed::Pin + Sized + 'static { /// Number of the pin within the port (0..31) #[inline] fn pin(&self) -> u8 { @@ -374,7 +374,7 @@ impl AnyPin { } } -impl_unborrow!(AnyPin); +unsafe_impl_unborrow!(AnyPin); impl Pin for AnyPin {} impl sealed::Pin for AnyPin { #[inline] @@ -469,7 +469,7 @@ impl OptionalPin for T { #[derive(Clone, Copy, Debug)] pub struct NoPin; -impl_unborrow!(NoPin); +unsafe_impl_unborrow!(NoPin); impl sealed::OptionalPin for NoPin {} impl OptionalPin for NoPin { type Pin = AnyPin; diff --git a/embassy-nrf/src/gpiote.rs b/embassy-nrf/src/gpiote.rs index 7e0220e79..2ef26e36a 100644 --- a/embassy-nrf/src/gpiote.rs +++ b/embassy-nrf/src/gpiote.rs @@ -5,7 +5,7 @@ use core::task::{Context, Poll}; use embassy::interrupt::{Interrupt, InterruptExt}; use embassy::traits::gpio::{WaitForAnyEdge, WaitForHigh, WaitForLow}; use embassy::util::AtomicWaker; -use embassy_extras::impl_unborrow; +use embassy_extras::unsafe_impl_unborrow; use embedded_hal::digital::v2::{InputPin, StatefulOutputPin}; use futures::future::poll_fn; @@ -382,7 +382,7 @@ pub trait Channel: sealed::Channel + Sized { pub struct AnyChannel { number: u8, } -impl_unborrow!(AnyChannel); +unsafe_impl_unborrow!(AnyChannel); impl sealed::Channel for AnyChannel {} impl Channel for AnyChannel { fn number(&self) -> usize { diff --git a/embassy-nrf/src/ppi.rs b/embassy-nrf/src/ppi.rs index e8bcbd603..c91a69c10 100644 --- a/embassy-nrf/src/ppi.rs +++ b/embassy-nrf/src/ppi.rs @@ -12,7 +12,7 @@ use core::marker::PhantomData; use core::ptr::NonNull; use embassy::util::Unborrow; -use embassy_extras::{impl_unborrow, unborrow}; +use embassy_extras::{unborrow, unsafe_impl_unborrow}; use crate::{pac, peripherals}; @@ -146,7 +146,7 @@ pub trait Group: sealed::Group + Sized { pub struct AnyChannel { number: u8, } -impl_unborrow!(AnyChannel); +unsafe_impl_unborrow!(AnyChannel); impl sealed::Channel for AnyChannel {} impl Channel for AnyChannel { fn number(&self) -> usize { @@ -157,7 +157,7 @@ impl Channel for AnyChannel { pub struct AnyConfigurableChannel { number: u8, } -impl_unborrow!(AnyConfigurableChannel); +unsafe_impl_unborrow!(AnyConfigurableChannel); impl sealed::Channel for AnyConfigurableChannel {} impl sealed::ConfigurableChannel for AnyConfigurableChannel {} impl ConfigurableChannel for AnyConfigurableChannel {} @@ -226,7 +226,7 @@ impl_channel!(PPI_CH31, 31); pub struct AnyGroup { number: u8, } -impl_unborrow!(AnyGroup); +unsafe_impl_unborrow!(AnyGroup); impl sealed::Group for AnyGroup {} impl Group for AnyGroup { fn number(&self) -> usize { diff --git a/embassy-nrf/src/saadc.rs b/embassy-nrf/src/saadc.rs index 65276a309..c1afd00de 100644 --- a/embassy-nrf/src/saadc.rs +++ b/embassy-nrf/src/saadc.rs @@ -190,7 +190,7 @@ impl<'d, T: PositivePin> Sample for OneShot<'d, T> { /// A pin that can be used as the positive end of a ADC differential in the SAADC periperhal. /// /// Currently negative is always shorted to ground (0V). -pub trait PositivePin { +pub trait PositivePin: Unborrow { fn channel(&self) -> PositiveChannel; } diff --git a/embassy-rp/src/gpio.rs b/embassy-rp/src/gpio.rs index ecfad1878..88d5d0936 100644 --- a/embassy-rp/src/gpio.rs +++ b/embassy-rp/src/gpio.rs @@ -6,7 +6,7 @@ use crate::pac::SIO; use crate::peripherals; use embassy::util::Unborrow; -use embassy_extras::{impl_unborrow, unborrow}; +use embassy_extras::{unborrow, unsafe_impl_unborrow}; use embedded_hal::digital::v2::{InputPin, OutputPin, StatefulOutputPin}; /// Represents a digital input or output level. @@ -209,7 +209,7 @@ pub trait Pin: sealed::Pin { pub struct AnyPin { pin_bank: u8, } -impl_unborrow!(AnyPin); +unsafe_impl_unborrow!(AnyPin); impl Pin for AnyPin {} impl sealed::Pin for AnyPin { fn pin_bank(&self) -> u8 { diff --git a/embassy-stm32/src/exti.rs b/embassy-stm32/src/exti.rs index fb1c6cd39..418e56fc0 100644 --- a/embassy-stm32/src/exti.rs +++ b/embassy-stm32/src/exti.rs @@ -7,7 +7,7 @@ use core::task::{Context, Poll}; use embassy::interrupt::{Interrupt, InterruptExt}; use embassy::traits::gpio::{WaitForAnyEdge, WaitForFallingEdge, WaitForRisingEdge}; use embassy::util::{AtomicWaker, Unborrow}; -use embassy_extras::impl_unborrow; +use embassy_extras::unsafe_impl_unborrow; use embedded_hal::digital::v2::InputPin; use pac::exti::{regs, vals}; @@ -214,7 +214,7 @@ pub trait Channel: sealed::Channel + Sized { pub struct AnyChannel { number: u8, } -impl_unborrow!(AnyChannel); +unsafe_impl_unborrow!(AnyChannel); impl sealed::Channel for AnyChannel {} impl Channel for AnyChannel { fn number(&self) -> usize { diff --git a/embassy-stm32/src/gpio.rs b/embassy-stm32/src/gpio.rs index d36d4a29d..9dc5858c8 100644 --- a/embassy-stm32/src/gpio.rs +++ b/embassy-stm32/src/gpio.rs @@ -2,7 +2,7 @@ use core::convert::Infallible; use core::marker::PhantomData; use embassy::util::Unborrow; -use embassy_extras::{impl_unborrow, unborrow}; +use embassy_extras::{unborrow, unsafe_impl_unborrow}; use embedded_hal::digital::v2::{InputPin, OutputPin, StatefulOutputPin}; use crate::pac; @@ -254,7 +254,7 @@ impl AnyPin { } } -impl_unborrow!(AnyPin); +unsafe_impl_unborrow!(AnyPin); impl Pin for AnyPin { type ExtiChannel = crate::exti::AnyChannel; } @@ -297,7 +297,7 @@ impl OptionalPin for T { #[derive(Clone, Copy, Debug)] pub struct NoPin; -impl_unborrow!(NoPin); +unsafe_impl_unborrow!(NoPin); impl sealed::OptionalPin for NoPin {} impl OptionalPin for NoPin { type Pin = AnyPin; diff --git a/embassy/src/util/mod.rs b/embassy/src/util/mod.rs index 8057e120e..9dd96b906 100644 --- a/embassy/src/util/mod.rs +++ b/embassy/src/util/mod.rs @@ -17,12 +17,12 @@ pub use portal::*; pub use signal::*; pub use waker::*; -pub trait Unborrow { +pub unsafe trait Unborrow { type Target; unsafe fn unborrow(self) -> Self::Target; } -impl<'a, T: Unborrow> Unborrow for &'a mut T { +unsafe impl<'a, T: Unborrow> Unborrow for &'a mut T { type Target = T::Target; unsafe fn unborrow(self) -> Self::Target { T::unborrow(core::ptr::read(self)) @@ -33,9 +33,9 @@ pub trait Steal { unsafe fn steal() -> Self; } -macro_rules! impl_unborrow_tuples { +macro_rules! unsafe_impl_unborrow_tuples { ($($t:ident),+) => { - impl<$($t),+> Unborrow for ($($t),+) + unsafe impl<$($t),+> Unborrow for ($($t),+) where $( $t: Unborrow @@ -51,14 +51,14 @@ macro_rules! impl_unborrow_tuples { }; } -impl_unborrow_tuples!(A, B); -impl_unborrow_tuples!(A, B, C); -impl_unborrow_tuples!(A, B, C, D); -impl_unborrow_tuples!(A, B, C, D, E); -impl_unborrow_tuples!(A, B, C, D, E, F); -impl_unborrow_tuples!(A, B, C, D, E, F, G); -impl_unborrow_tuples!(A, B, C, D, E, F, G, H); -impl_unborrow_tuples!(A, B, C, D, E, F, G, H, I); -impl_unborrow_tuples!(A, B, C, D, E, F, G, H, I, J); -impl_unborrow_tuples!(A, B, C, D, E, F, G, H, I, J, K); -impl_unborrow_tuples!(A, B, C, D, E, F, G, H, I, J, K, L); +unsafe_impl_unborrow_tuples!(A, B); +unsafe_impl_unborrow_tuples!(A, B, C); +unsafe_impl_unborrow_tuples!(A, B, C, D); +unsafe_impl_unborrow_tuples!(A, B, C, D, E); +unsafe_impl_unborrow_tuples!(A, B, C, D, E, F); +unsafe_impl_unborrow_tuples!(A, B, C, D, E, F, G); +unsafe_impl_unborrow_tuples!(A, B, C, D, E, F, G, H); +unsafe_impl_unborrow_tuples!(A, B, C, D, E, F, G, H, I); +unsafe_impl_unborrow_tuples!(A, B, C, D, E, F, G, H, I, J); +unsafe_impl_unborrow_tuples!(A, B, C, D, E, F, G, H, I, J, K); +unsafe_impl_unborrow_tuples!(A, B, C, D, E, F, G, H, I, J, K, L); From 1c0ad538410b61f8011c7d8facd136a85138be61 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Wed, 19 May 2021 23:38:29 +0200 Subject: [PATCH 3/3] Unborrow docs --- embassy/src/util/mod.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/embassy/src/util/mod.rs b/embassy/src/util/mod.rs index 9dd96b906..88ae5c285 100644 --- a/embassy/src/util/mod.rs +++ b/embassy/src/util/mod.rs @@ -17,8 +17,27 @@ pub use portal::*; pub use signal::*; pub use waker::*; +/// Unsafely unborrow an owned singleton out of a `&mut`. +/// +/// It is intended to be implemented for owned peripheral singletons, such as `USART3` or `AnyPin`. +/// Unborrowing an owned `T` yields the same `T`. Unborrowing a `&mut T` yields a copy of the T. +/// +/// This allows writing HAL drivers that either own or borrow their peripherals, but that don't have +/// to store pointers in the borrowed case. +/// +/// Safety: this trait can be used to copy non-Copy types. Implementors must not cause +/// immediate UB when copied, and must not cause UB when copies are later used, provided they +/// are only used according the [`Self::unborrow`] safety contract. +/// pub unsafe trait Unborrow { + /// Unborrow result type type Target; + + /// Unborrow a value. + /// + /// Safety: This returns a copy of a singleton that's normally not + /// copiable. The returned copy must ONLY be used while the lifetime of `self` is + /// valid, as if it were accessed through `self` every time. unsafe fn unborrow(self) -> Self::Target; }