Use sealed traits to prevent marker trait implementations on arbitrar…

…y types
rust-embedded-community · Jul 6, 2022 · 93e1ed9 · 93e1ed9
1 parent f8a7efb
commit 93e1ed9
Show file tree

Hide file tree

Showing 9 changed files with 148 additions and 119 deletions.
diff --git a/tm4c-hal/src/i2c.rs b/tm4c-hal/src/i2c.rs
@@ -28,14 +28,14 @@ macro_rules! i2c_pins {
         sda: [$(($($sdagpio: ident)::*, $sdaaf: ident)),*],
     ) => {
         $(
-            unsafe impl<T> SclPin<$I2Cn> for $($sclgpio)::*<AlternateFunction<$sclaf, T>>
+            impl<T> SclPin<$I2Cn> for $($sclgpio)::*<AlternateFunction<$sclaf, T>>
             where
                 T: OutputMode,
             {}
         )*
 
         $(
-            unsafe impl<T> SdaPin<$I2Cn> for $($sdagpio)::*<AlternateFunction<$sdaaf, T>>
+            impl<T> SdaPin<$I2Cn> for $($sdagpio)::*<AlternateFunction<$sdaaf, T>>
             where
                 T: OutputMode,
             {}

diff --git a/tm4c-hal/src/lib.rs b/tm4c-hal/src/lib.rs
@@ -191,6 +191,8 @@ macro_rules! gpio_macro {
                     _mode: PhantomData<MODE>,
                 }
 
+                impl<MODE> crate::Sealed for $PXi<MODE> {}
+
                 impl<MODE> $PXi<MODE> where MODE: IsUnlocked {
                     /// Configures the pin to serve as alternate function 1 through 15.
                     /// Disables open-drain to make the output a push-pull.
@@ -525,6 +527,8 @@ macro_rules! uart_hal_macro {
     ($(
         $UARTX:ident: ($powerDomain:ident, $uartX:ident),
     )+) => {
+        $crate::uart_traits_macro!();
+
         $(
             impl<TX, RX, RTS, CTS> Serial<$UARTX, TX, RX, RTS, CTS> {
                 /// Configures a UART peripheral to provide serial communication
@@ -780,7 +784,7 @@ macro_rules! uart_pin_macro {
         tx: [$(($($txgpio: ident)::*, $txaf: ident)),*],
     ) => {
         $(
-            unsafe impl<T> CtsPin<$UARTn> for $($ctsgpio)::*<AlternateFunction<$ctsaf, T>>
+            impl<T> CtsPin<$UARTn> for $($ctsgpio)::*<AlternateFunction<$ctsaf, T>>
             where
                 T: OutputMode,
             {
@@ -791,7 +795,7 @@ macro_rules! uart_pin_macro {
         )*
 
         $(
-            unsafe impl<T> RtsPin<$UARTn> for $($rtsgpio)::*<AlternateFunction<$rtsaf, T>>
+            impl<T> RtsPin<$UARTn> for $($rtsgpio)::*<AlternateFunction<$rtsaf, T>>
             where
                 T: OutputMode,
             {
@@ -802,14 +806,14 @@ macro_rules! uart_pin_macro {
         )*
 
         $(
-            unsafe impl <T> RxPin<$UARTn> for $($rxgpio)::*<AlternateFunction<$rxaf, T>>
+            impl <T> RxPin<$UARTn> for $($rxgpio)::*<AlternateFunction<$rxaf, T>>
             where
                 T: OutputMode,
             {}
         )*
 
         $(
-            unsafe impl <T> TxPin<$UARTn> for $($txgpio)::*<AlternateFunction<$txaf, T>>
+            impl <T> TxPin<$UARTn> for $($txgpio)::*<AlternateFunction<$txaf, T>>
             where
                 T: OutputMode,
             {}

diff --git a/tm4c-hal/src/serial.rs b/tm4c-hal/src/serial.rs
@@ -1,80 +1,89 @@
 //! Serial code that is generic to both the TM4C123 and TM4C129, such as the pin traits.
 
-/// TX pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait TxPin<UART> {}
+// The macro is required for the "sealed trait" pattern to work:
+// the traits and the gpios have to be defined in the same crate
 
-/// RX pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait RxPin<UART> {}
+///! An internal macro to generate the UART traits
+#[macro_export]
+macro_rules! uart_traits_macro {
+    () => {
+        /// TX pin
+        pub trait TxPin<UART>: crate::Sealed {}
 
-/// CTS pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait CtsPin<UART> {
-    /// Enables the CTS functionality if a valid pin is given (not `()`).
-    fn enable(&mut self, _uart: &mut UART);
-}
+        /// RX pin
+        pub trait RxPin<UART>: crate::Sealed {}
 
-/// DCD pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait DcdPin<UART> {
-    /// Enables the DCD functionality if a valid pin is given (not `()`).
-    fn enable(&mut self, _uart: &mut UART);
-}
+        /// CTS pin
+        pub trait CtsPin<UART>: crate::Sealed {
+            /// Enables the CTS functionality if a valid pin is given (not `()`).
+            fn enable(&mut self, _uart: &mut UART);
+        }
 
-/// DSR pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait DsrPin<UART> {
-    /// Enables the DSR functionality if a valid pin is given (not `()`).
-    fn enable(&mut self, _uart: &mut UART);
-}
+        /// DCD pin
+        pub trait DcdPin<UART>: crate::Sealed {
+            /// Enables the DCD functionality if a valid pin is given (not `()`).
+            fn enable(&mut self, _uart: &mut UART);
+        }
 
-/// DTR pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait DtrPin<UART> {
-    /// Enables the DTR functionality if a valid pin is given (not `()`).
-    fn enable(&mut self, _uart: &mut UART);
-}
+        /// DSR pin
+        pub trait DsrPin<UART>: crate::Sealed {
+            /// Enables the DSR functionality if a valid pin is given (not `()`).
+            fn enable(&mut self, _uart: &mut UART);
+        }
 
-/// RI pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait RiPin<UART> {
-    /// Enables the RI functionality if a valid pin is given (not `()`).
-    fn enable(&mut self, _uart: &mut UART);
-}
+        /// DTR pin
+        pub trait DtrPin<UART>: crate::Sealed {
+            /// Enables the DTR functionality if a valid pin is given (not `()`).
+            fn enable(&mut self, _uart: &mut UART);
+        }
 
-/// RTS pin - DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait RtsPin<UART> {
-    /// Enables the RTS functionality if a valid pin is given (not `()`).
-    fn enable(&mut self, _uart: &mut UART);
-}
+        /// RI pin
+        pub trait RiPin<UART>: crate::Sealed {
+            /// Enables the RI functionality if a valid pin is given (not `()`).
+            fn enable(&mut self, _uart: &mut UART);
+        }
 
-unsafe impl<U> TxPin<U> for () {}
+        /// RTS pin
+        pub trait RtsPin<UART>: crate::Sealed {
+            /// Enables the RTS functionality if a valid pin is given (not `()`).
+            fn enable(&mut self, _uart: &mut UART);
+        }
 
-unsafe impl<U> RxPin<U> for () {}
+        impl<U> TxPin<U> for () {}
 
-unsafe impl<U> CtsPin<U> for () {
-    fn enable(&mut self, _uart: &mut U) {
-        // Do nothing
-    }
-}
-unsafe impl<U> DcdPin<U> for () {
-    fn enable(&mut self, _uart: &mut U) {
-        // Do nothing
-    }
-}
-unsafe impl<U> DsrPin<U> for () {
-    fn enable(&mut self, _uart: &mut U) {
-        // Do nothing
-    }
-}
-unsafe impl<U> DtrPin<U> for () {
-    fn enable(&mut self, _uart: &mut U) {
-        // Do nothing
-    }
-}
-unsafe impl<U> RiPin<U> for () {
-    fn enable(&mut self, _uart: &mut U) {
-        // Do nothing
-    }
-}
-unsafe impl<U> RtsPin<U> for () {
-    fn enable(&mut self, _uart: &mut U) {
-        // Do nothing
-    }
+        impl<U> RxPin<U> for () {}
+
+        impl<U> CtsPin<U> for () {
+            fn enable(&mut self, _uart: &mut U) {
+                // Do nothing
+            }
+        }
+        impl<U> DcdPin<U> for () {
+            fn enable(&mut self, _uart: &mut U) {
+                // Do nothing
+            }
+        }
+        impl<U> DsrPin<U> for () {
+            fn enable(&mut self, _uart: &mut U) {
+                // Do nothing
+            }
+        }
+        impl<U> DtrPin<U> for () {
+            fn enable(&mut self, _uart: &mut U) {
+                // Do nothing
+            }
+        }
+        impl<U> RiPin<U> for () {
+            fn enable(&mut self, _uart: &mut U) {
+                // Do nothing
+            }
+        }
+        impl<U> RtsPin<U> for () {
+            fn enable(&mut self, _uart: &mut U) {
+                // Do nothing
+            }
+        }
+    };
 }
 
 /// writeln!() emits LF chars, so this is useful

diff --git a/tm4c123x-hal/src/i2c.rs b/tm4c123x-hal/src/i2c.rs
@@ -5,6 +5,7 @@ use crate::{
     hal::blocking::i2c::{Read, Write, WriteRead},
     sysctl::{self, Clocks},
     time::Hertz,
+    Sealed,
 };
 
 use cortex_m::asm::delay;
@@ -21,12 +22,11 @@ pub struct I2c<I2C, PINS> {
     pub pins: PINS,
 }
 
-// FIXME these should be "closed" traits
-/// SCL pin -- DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait SclPin<I2C> {}
+/// SCL pin
+pub trait SclPin<I2C>: Sealed {}
 
-/// SDA pin -- DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait SdaPin<I2C> {}
+/// SDA pin
+pub trait SdaPin<I2C>: Sealed {}
 
 i2c_pins!(I2C0, scl: [(gpiob::PB2, AF3)], sda: [(gpiob::PB3, AF3)],);
 i2c_pins!(I2C1, scl: [(gpioa::PA6, AF3)], sda: [(gpioa::PA7, AF3)],);

diff --git a/tm4c123x-hal/src/lib.rs b/tm4c123x-hal/src/lib.rs
@@ -32,6 +32,14 @@ pub use crate::tm4c123x::interrupt;
 
 use embedded_hal as hal;
 
+use sealed::Sealed;
+mod sealed {
+    // To prevent implementation of `*Pin` traits on arbitrary types
+    pub trait Sealed {}
+
+    impl Sealed for () {}
+}
+
 pub mod gpio;
 pub mod hib;
 pub mod i2c;

diff --git a/tm4c123x-hal/src/spi.rs b/tm4c123x-hal/src/spi.rs
@@ -13,6 +13,7 @@ use crate::{
     sysctl,
     sysctl::Clocks,
     time::Hertz,
+    Sealed,
 };
 
 use tm4c123x::{SSI0, SSI1, SSI2, SSI3};
@@ -24,35 +25,34 @@ pub enum Error {
     _Extensible,
 }
 
-// FIXME these should be "closed" traits
-/// SCK pin -- DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait SckPin<SPI> {}
+/// SCK pin
+pub trait SckPin<SPI>: Sealed {}
 
-/// MISO pin -- DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait MisoPin<SPI> {}
+/// MISO pin
+pub trait MisoPin<SPI>: Sealed {}
 
-/// MOSI pin -- DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait MosiPin<SPI> {}
+/// MOSI pin
+pub trait MosiPin<SPI>: Sealed {}
 
 // SSI0
-unsafe impl<T> SckPin<SSI0> for PA2<AlternateFunction<AF2, T>> where T: OutputMode {}
-unsafe impl<T> MisoPin<SSI0> for PA4<AlternateFunction<AF2, T>> where T: OutputMode {}
-unsafe impl<T> MosiPin<SSI0> for PA5<AlternateFunction<AF2, T>> where T: OutputMode {}
+impl<T> SckPin<SSI0> for PA2<AlternateFunction<AF2, T>> where T: OutputMode {}
+impl<T> MisoPin<SSI0> for PA4<AlternateFunction<AF2, T>> where T: OutputMode {}
+impl<T> MosiPin<SSI0> for PA5<AlternateFunction<AF2, T>> where T: OutputMode {}
 
 // SSI1
-unsafe impl<T> SckPin<SSI1> for PD0<AlternateFunction<AF2, T>> where T: OutputMode {}
-unsafe impl<T> MisoPin<SSI1> for PD2<AlternateFunction<AF2, T>> where T: OutputMode {}
-unsafe impl<T> MosiPin<SSI1> for PD3<AlternateFunction<AF2, T>> where T: OutputMode {}
+impl<T> SckPin<SSI1> for PD0<AlternateFunction<AF2, T>> where T: OutputMode {}
+impl<T> MisoPin<SSI1> for PD2<AlternateFunction<AF2, T>> where T: OutputMode {}
+impl<T> MosiPin<SSI1> for PD3<AlternateFunction<AF2, T>> where T: OutputMode {}
 
 // SSI2
-unsafe impl<T> SckPin<SSI2> for PB4<AlternateFunction<AF2, T>> where T: OutputMode {}
-unsafe impl<T> MisoPin<SSI2> for PB6<AlternateFunction<AF2, T>> where T: OutputMode {}
-unsafe impl<T> MosiPin<SSI2> for PB7<AlternateFunction<AF2, T>> where T: OutputMode {}
+impl<T> SckPin<SSI2> for PB4<AlternateFunction<AF2, T>> where T: OutputMode {}
+impl<T> MisoPin<SSI2> for PB6<AlternateFunction<AF2, T>> where T: OutputMode {}
+impl<T> MosiPin<SSI2> for PB7<AlternateFunction<AF2, T>> where T: OutputMode {}
 
 // SSI3
-unsafe impl<T> SckPin<SSI3> for PD0<AlternateFunction<AF1, T>> where T: OutputMode {}
-unsafe impl<T> MisoPin<SSI3> for PD2<AlternateFunction<AF1, T>> where T: OutputMode {}
-unsafe impl<T> MosiPin<SSI3> for PD3<AlternateFunction<AF1, T>> where T: OutputMode {}
+impl<T> SckPin<SSI3> for PD0<AlternateFunction<AF1, T>> where T: OutputMode {}
+impl<T> MisoPin<SSI3> for PD2<AlternateFunction<AF1, T>> where T: OutputMode {}
+impl<T> MosiPin<SSI3> for PD3<AlternateFunction<AF1, T>> where T: OutputMode {}
 
 /// SPI peripheral operating in full duplex master mode
 pub struct Spi<SPI, PINS> {

diff --git a/tm4c129x-hal/src/i2c.rs b/tm4c129x-hal/src/i2c.rs
@@ -5,6 +5,7 @@ use crate::{
     hal::blocking::i2c::{Read, Write, WriteRead},
     sysctl::{self, Clocks},
     time::Hertz,
+    Sealed,
 };
 
 use cortex_m::asm::delay;
@@ -21,12 +22,11 @@ pub struct I2c<I2C, PINS> {
     pub pins: PINS,
 }
 
-// FIXME these should be "closed" traits
-/// SCL pin -- DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait SclPin<I2C> {}
+/// SCL pin
+pub trait SclPin<I2C>: Sealed {}
 
-/// SDA pin -- DO NOT IMPLEMENT THIS TRAIT
-pub unsafe trait SdaPin<I2C> {}
+/// SDA pin
+pub trait SdaPin<I2C>: Sealed {}
 
 i2c_pins!(I2C0, scl: [(gpiob::PB2, AF2)], sda: [(gpiob::PB3, AF2)],);
 i2c_pins!(I2C1, scl: [(gpiog::PG0, AF2)], sda: [(gpiog::PG1, AF2)],);

diff --git a/tm4c129x-hal/src/lib.rs b/tm4c129x-hal/src/lib.rs
@@ -32,6 +32,14 @@ pub use tm4c_hal::{bb, delay, time};
 #[cfg(feature = "rt")]
 pub use crate::tm4c129x::interrupt;
 
+use sealed::Sealed;
+mod sealed {
+    // To prevent implementation of `*Pin` traits on arbitrary types
+    pub trait Sealed {}
+
+    impl Sealed for () {}
+}
+
 pub mod gpio;
 pub mod hib;
 pub mod i2c;