Merge pull request #195 from embassy-rs/unborrow-mk2

Unborrow fixes

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 {
@@ -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)
-                    }
-                }
             )*
         }
 
@@ -86,23 +78,15 @@ 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 {
                 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) }
-            }
-        }
     };
 }
 

embassy-macros/src/lib.rs

@@ -210,19 +210,12 @@ 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
             }
         }
-
-        impl ::embassy::util::Unborrow for &mut #name_interrupt {
-            type Target = #name_interrupt;
-            unsafe fn unborrow(self) -> #name_interrupt {
-                ::core::ptr::read(self)
-            }
-        }
     };
     result.into()
 }

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<Target = Self> + sealed::Pin + Sized {
+pub trait Pin: Unborrow<Target = Self> + 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<T: Pin> 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;

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 {

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 {

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<Target = Self> {
     fn channel(&self) -> PositiveChannel;
 }
 

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 {

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 {

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<T: Pin> 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;

embassy/src/util/mod.rs

@@ -17,18 +17,44 @@ pub use portal::*;
 pub use signal::*;
 pub use waker::*;
 
-pub trait Unborrow {
+/// 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;
 }
 
+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))
+    }
+}
+
 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<Target = $t>
@@ -40,29 +66,18 @@ macro_rules! impl_unborrow_tuples {
             }
         }
 
-        impl<'a, $($t),+> Unborrow for &'a mut($($t),+)
-        where
-            $(
-                $t: Unborrow<Target = $t>
-            ),+
-        {
-            type Target = ($($t),+);
-            unsafe fn unborrow(self) -> Self::Target {
-                ::core::ptr::read(self)
-            }
-        }
 
     };
 }
 
-impl_unborrow_tuples!(A, B);
+unsafe_impl_unborrow_tuples!(A, B);
-impl_unborrow_tuples!(A, B, C);
+unsafe_impl_unborrow_tuples!(A, B, C);
-impl_unborrow_tuples!(A, B, C, D);
+unsafe_impl_unborrow_tuples!(A, B, C, D);
-impl_unborrow_tuples!(A, B, C, D, E);
+unsafe_impl_unborrow_tuples!(A, B, C, D, E);
-impl_unborrow_tuples!(A, B, C, D, E, F);
+unsafe_impl_unborrow_tuples!(A, B, C, D, E, F);
-impl_unborrow_tuples!(A, B, C, D, E, F, G);
+unsafe_impl_unborrow_tuples!(A, B, C, D, E, F, G);
-impl_unborrow_tuples!(A, B, C, D, E, F, G, H);
+unsafe_impl_unborrow_tuples!(A, B, C, D, E, F, G, H);
-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);
-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);
-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);
-impl_unborrow_tuples!(A, B, C, D, E, F, G, H, I, J, K, L);
+unsafe_impl_unborrow_tuples!(A, B, C, D, E, F, G, H, I, J, K, L);