Drop the `peripheral_trait!` macro

### what

#### Background

I.e. this macro:
https://github.com/ut-utp/core/blob/5e0f0eb43de2fe9e94f124cff6bc7db0821c7737/traits/src/peripherals/mod.rs#L196-L358

which is invoked on all the peripheral trait definitions:
https://github.com/ut-utp/core/blob/5e0f0eb43de2fe9e94f124cff6bc7db0821c7737/traits/src/peripherals/gpio.rs#L141

Like #176 this was a well-intentioned but ultimately ill-advised early design decision.

The idea here was to have a `Peripherals` trait that is a super trait of all of the individual peripheral traits, for a few reasons:
  1) so that users of the API could simply ask for a `P: Peripherals` instance instead of needing to grow 7+ generic parameters
  2) so that users of peripheral instances could just operate on a single instance, i.e. `Gpio::read(p, ...)`, `Clock::get_milliseconds(p, ...)`. This is particularly useful [in tests](https://github.com/ut-utp/core/blob/ef52e746987ede58b68eff07dcf2be14f5e5d5ed/baseline-sim/tests/device_register_tests/mem_mapped/adc.rs#L108)
  3) so that we don't _require_ peripheral implementations that want to share state with other peripherals (i.e. an `Input` impl and an `Output` impl that share state to act as a loopback device) to use _external_ state sharing mechanisms (i.e. `Arc<Mutex<_>>`, `RefCell<_>`, etc.)

The 3rd point is a little subtle; here's an example:
  - under the "obvious" paradigm of having peripheral-trait-users be generic over the individual peripheral traits, you end up with users of the traits having types that look like the `PeripheralsSet`:
    ```rust
    struct MyNeatType<I: Input, O: Output, ...> {
      input: I,
      output: O,
      ...
    }
    ```
  - the issue with the above is that it _requires_ `input` and `output` to be separate instances; if we want one type (i.e. `LoopbackIo`) and one _instance_ of that type to provide both our `Input` and `Output` implementations we need to resort to "external" state sharing mechanisms like `Arc<Mutex<_>>` to produce two instances that ultimately reference the same instance _or_ some shared data
  - having a `Peripherals` supertrait on the other hand lets us sidestep this:
     + the `Peripherals` implementor can _choose_ to shell out to another type for a particular peripheral trait
     + but nothing requires it to shell out to _separate_ types (or instances) for each peripheral trait that it is a super trait of

For the above reasons, we made `Peripherals` a supertrait:
https://github.com/ut-utp/core/blob/5e0f0eb43de2fe9e94f124cff6bc7db0821c7737/traits/src/peripherals/mod.rs#L37

We still recognized that the common case was definitely going to be separate instances for each peripheral and that it would be unacceptable for users to implement `Peripherals` for each unique set of peripheral trait implementors they wanted to use together, so:
  - we created `PeripheralSet` which held instances of each peripheral and was generic over the 7 peripherals
  - we had `PeripheralSet` _implement_ each of the peripheral traits by shelling out to the peripheral instance contained within (this is what the `peripheral_trait!` macro did)

#### Problems

This worked but had several downsides, mostly with the `peripheral_trait` macro:
  - it's super brittle! the [`peripheral_trait` macro](https://github.com/ut-utp/core/blob/5e0f0eb43de2fe9e94f124cff6bc7db0821c7737/traits/src/peripherals/mod.rs#L196-L358) contains an ad-hoc redefinition of the Rust grammar's definition of what constitutes legal syntax for trait definitions: it's both very hard to read _and_ subtly wrong in many ways
  - bad developer experience: rust-analyzer and other tooling (i.e. `rustfmt`) don't interact well with items declared inside declarative macros like this. errors stemming from mistakes in the declarative macro's definition of trait definition syntax are extremely hard to diagnose for developers not very familiar with Rust declarative macros
  - it doesn't compose well; we cannot, for example, do what's described in #... with `peripheral_trait!` because we can't have a declarative macro like it run on the _output_ of a macro like `#[async_trait]`

#### A Solution

3 years later, with the benefit of hindsight, we can come up with a solution that sidesteps these issues while still fulfilling the design constraints that led us to write `peripheral_trait`.

##### `Peripherals` trait

First: We should have the `Peripherals` trait have _associated types_ for each of the peripheral traits within instead of being a super trait. Something like:

```rust
pub trait Peripherals {
    type Gpio: Gpio;
    type Adc: Adc;
    type Pwm: Pwm;
    type Timers: Timers;
    type Clock: Clock;
    type Input: Input;
    type Output: Output;

    fn get_gpio(&self) -> &Self::Gpio;
    fn get_gpio_mut(&mut self) -> &mut Self::Gpio;

    fn get_adc(&self) -> &Self::Adc;
    fn get_adc_mut(&mut self) -> &mut Self::Adc;

    fn get_pwm(&self) -> &Self::Pwm;
    fn get_pwm_mut(&mut self) -> &mut Self::Pwm;

    fn get_timers(&self) -> &Self::Timers;
    fn get_timers_mut(&mut self) -> &mut Self::Timers;

    fn get_clock(&self) -> &Self::Clock;
    fn get_clock_mut(&mut self) -> &mut Self::Clock;

    fn get_input(&self) -> &Self::Input;
    fn get_input_mut(&mut self) -> &mut Self::Input;

    fn get_output(&self) -> &Self::Output;
    fn get_output_mut(&mut self) -> &mut Self::Output;
}
```

Our goal was to hide the particular types used for the peripherals instead of having them leak into users of the peripherals as generic parameters. Associated types are a cleaner way of achieving this than supertraits.

As an added bonus it's now easier to _selectively_ have some peripherals be shared; i.e. if you want to have a `LoopbackIo` device, you only need to:
  - create a struct holding your `LoopbackIo` device and whatever other peripherals you want to use
  - implement `Peripherals` on that struct, exposing the same type and instance for `Input` and `Output` and their getters

Under the existing setup you would have had to write delegating trait implementations of all the peripheral traits for your new type (with the `Input` and `Output` impls delegating to the same instance).

##### `PeripheralsWrapper`

The above addresses the "shared state between peripheral impls" concern (the 3rd point) and manages to make using the Peripherals in your own API not overly cumbersome (you can just be generic over `P: Peripherals` instead of all 7 individual peripheral traits) but it does not address the 2nd concern: actually using this version of the `Peripherals` trait is still more cumbersome that the current incarnation because you now need to call `get_gpio`, etc. before calling your particular peripheral trait's methods.

We can fix this by: providing a type that _does_ have all the peripheral traits implemented on it and delegates to an underlying `Peripherals` instance.

I think we had kind of the right _idea_ with `peripheral_trait!` but just bad execution: it's hard to make a _declarative_ macro like this robust and composable but _proc macro_ attributes are a good fit for this task. [`ambassador`](https://docs.rs/ambassador/) is one such proc macro that does exactly what `peripheral_trait` was looking to do.

We can use `ambassador` to provide delegated impls of all the peripheral traits on `PeripheralsWrapper`, where `PeripheralsWrapper` is a type like:
```rust
#[repr(transparent)]
struct PeripheralsWrapper<P: Peripherals>(P);
```
with delegations like this:
```rust
#[ambassador::delegate_to_methods]
#[delegate(Gpio, target_ref = "get_gpio", target_mut = "get_gpio_mut")]
#[delegate(Adc, target_ref = "get_adc", target_mut = "get_adc_mut")]
#[delegate(Pwm, target_ref = "get_pwm", target_mut = "get_pwm_mut")]
#[delegate(Timers, target_ref = "get_timers", target_mut = "get_timers_mut")]
#[delegate(Clock, target_ref = "get_clock", target_mut = "get_clock_mut")]
#[delegate(Input, target_ref = "get_input", target_mut = "get_input_mut")]
#[delegate(Output, target_ref = "get_output", target_mut = "get_output_mut")]
impl<P: Peripherals> PeripheralsWrapper<'_, P> {
    fn get_gpio(&self) -> &P::Gpio { self.0.get_gpio() }
    fn get_gpio_mut(&mut self) -> &mut P::Gpio { self.0.get_gpio_mut() }
    fn get_adc(&self) -> &P::Adc { self.0.get_adc() }
    fn get_adc_mut(&mut self) -> &mut P::Adc { self.0.get_adc_mut() }
    fn get_pwm(&self) -> &P::Pwm { self.0.get_pwm() }
    fn get_pwm_mut(&mut self) -> &mut P::Pwm { self.0.get_pwm_mut() }
    fn get_timers(&self) -> &P::Timers { self.0.get_timers() }
    fn get_timers_mut(&mut self) -> &mut P::Timers { self.0.get_timers_mut() }
    fn get_clock(&self) -> &P::Clock { self.0.get_clock() }
    fn get_clock_mut(&mut self) -> &mut P::Clock { self.0.get_clock_mut() }
    fn get_input(&self) -> &P::Input { self.0.get_input() }
    fn get_input_mut(&mut self) -> &mut P::Input { self.0.get_input_mut() }
    fn get_output(&self) -> &P::Output { self.0.get_output() }
    fn get_output_mut(&mut self) -> &mut P::Output { self.0.get_output_mut() }
}
```

> *warning*
> Note:
>   - we'd like to just implement, for example `<P: Peripherals> Gpio for P` but `ambassador` doesn't have a way to do this so we use the `PeripheralsWrapper` newtype

We can then offer this extension to actually construct a `PeripheralsWrapper` without consuming the underlying `Peripherals` instance:
```rust
pub trait PeripheralsExt: Peripherals {
    /// Gets you a wrapper type that impls all the traits.
    fn peripherals_wrapper(&self) -> &'_ PeripheralsWrapper<Self> {
      unsafe { core::mem::transmute(self) } // safe because of `repr(transparent)`
    }
   fn peripherals_wrapper_mut(&mut self) -> &'_ mut PeripheralsWrapper<Self> { ... }
}

impl<P: Peripherals> PeripheralsExt for P { }
```

And have the `Deref`/`DerefMut` impls on `InstructionInterpreter` leverage the above methods.

We can also provide delegated impls on `PeripheralSet` to cover the common case.

### steps
 - [ ] revise the `Peripherals` trait
 - [ ] drop the `peripheral_trait!` macro
 - [ ] introduce the `ambassador` provided delegated impls

### where
branch: `imp/peripheral-trait-reform`

### open questions


	#[doc(hidden)]
	#[macro_export]
	macro_rules! peripheral_trait {
	($nom:ident, $(#[$attr:meta])* pub trait $trait:ident $(<$lifetime:lifetime>)? $(: $bound:ident )? { $($rest:tt)* }) => {
	$(#[$attr])*
	pub trait $trait $(<$lifetime>)? where Self: $($bound)? { $($rest)* }

	// $crate::deref_impl!($trait$(<$lifetime>)? $(\| $lifetime \|)?, { $($rest)* });
	// $crate::borrow_impl!($trait$(<$lifetime>)? $(\| $lifetime \|)?, { $($rest)* });
	$crate::peripheral_set_impl!($trait$(<$lifetime>)? $(\| $lifetime \|)?, { $crate::func_sig!($nom, $($rest)*); });
	// $crate::peripheral_deref_set_impl!($trait$(<$lifetime>)? $(\| $lifetime \|)?, { $crate::func_sig!($nom, $($rest)*); });
	};
	}

	#[doc(hidden)]
	#[macro_export]
	macro_rules! deref_impl {
	($trait:path $(\| $lifetime:lifetime \|)?, { $($rest:tt)* }) => {
	#[allow(unnecessary_qualification)]
	impl<$($lifetime,)? I, T: Default + core::ops::Deref<Target = I> + core::ops::DerefMut> $trait for T
	where
	I: $trait,
	{ $crate::func_sig!(+(), $($rest)); }
	};
	}

	#[doc(hidden)]
	#[macro_export]
	macro_rules! borrow_impl {
	($trait:path $(\| $lifetime:lifetime \|)?, { $($rest:tt)* }) => {
	#[allow(unnecessary_qualification)]
	impl<$($lifetime,)? I, T: Default + core::ops::Deref<Target = I> + core::ops::DerefMut + core::borrow::Borrow<I> + core::borrow::BorrowMut<I>> $trait for T
	where
	I: $trait,
	{ $crate::func_sig!(%(borrow, borrow_mut), $($rest)*); }
	};
	}

	#[doc(hidden)]
	#[macro_export]
	macro_rules! peripheral_set_impl {
	($trait:ty $(\| $lifetime:lifetime \|)?, { $($rest:tt)* }) => {
	impl<$($lifetime,)? 'p, G, A, P, T, C, I, O> $trait for $crate::peripherals::PeripheralSet<'p, G, A, P, T, C, I, O/, G, A, P, T, C, I, O/>
	where
	$($lifetime: 'p,)?
	G: $crate::peripherals::gpio::Gpio<'p>,
	A: $crate::peripherals::adc::Adc,
	P: $crate::peripherals::pwm::Pwm,
	T: $crate::peripherals::timers::Timers<'p>,
	C: $crate::peripherals::clock::Clock,
	I: $crate::peripherals::input::Input<'p>,
	O: $crate::peripherals::output::Output<'p>,
	{ $($rest)* }
	};
	}

	// #[doc(hidden)]
	// #[macro_export]
	// macro_rules! peripheral_deref_set_impl {
	// ($trait:ty $(\| $lifetime:lifetime \|)?, { $($rest:tt)* }) => {
	// impl<$($lifetime,)? 'p, G, A, P, T, C, I, O, GInner, AInner, PInner, TInner, CInner, IInner, OInner> $trait for $crate::peripherals::PeripheralSet<'p, G, A, P, T, C, I, O>
	// where
	// $($lifetime: 'p,)?
	// G: 'p + $crate::peripherals::gpio::Gpio<'p>,
	// A: 'p + $crate::peripherals::adc::Adc,
	// P: 'p + $crate::peripherals::pwm::Pwm,
	// T: 'p + $crate::peripherals::timers::Timers<'p>,
	// C: 'p + $crate::peripherals::clock::Clock,
	// I: 'p + $crate::peripherals::input::Input<'p>,
	// O: 'p + $crate::peripherals::output::Output<'p>,
	// GInner: 'p + Deref<
	// { $($rest)* }
	// };
	// }

	#[doc(hidden)]
	#[macro_export]
	macro_rules! func_sig {
	// [No block + Ret] Our ideal form: specified return type, no block:
	// (none)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident($($idents:ident : $types:ty),) -> $ret:ty; $($rest:tt)) => {
	#[inline] fn $fn_name($($idents : $types),*) -> $ret { compile_error!("trait functions not supported yet!") }
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, $($rest)*);
	};
	// (self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(self$(,)? $($idents:ident : $types:ty),) -> $ret:ty; $($rest:tt)) => {
	#[inline] fn $fn_name(self, $($idents : $types),) -> $ret { ($($indir$indir)?self)$(.$nom)?$(.$i_im())?.$fn_name($($idents),) }
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, $($rest)*);
	};
	// (mut self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(mut self$(,)? $($idents:ident : $types:ty),) -> $ret:ty; $($rest:tt)) => {
	#[inline] fn $fn_name(mut self, $($idents : $types),) -> $ret { ($($indir$indir)?self)$(.$nom)?$(.$i_mut())?.$fn_name($($idents),) }
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, $($rest)*);
	};
	// (&self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(&self$(,)? $($idents:ident : $types:ty),) -> $ret:ty; $($rest:tt)) => {
	#[inline] fn $fn_name(&self, $($idents : $types),) -> $ret { ($($indir$indir)?self)$(.$nom)?$(.$i_im())?.$fn_name($($idents),) }
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, $($rest)*);
	};
	// (&mut self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(&mut self$(,)? $($idents:ident : $types:ty),) -> $ret:ty; $($rest:tt)) => {
	#[inline] fn $fn_name(&mut self, $($idents : $types),) -> $ret { ($($indir$indir)?self)$(.$nom)?$(.$i_mut())?.$fn_name($($idents),) }
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, $($rest)*);
	};


	// [Block + Ret] Ditch blocks if you've got them:
	// (none)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident($($idents:ident : $types:ty),) -> $ret:ty $block:block $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name($($idents : $types),) -> $ret; $($rest)); };
	// (self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(self$(,)? $($self:expr,)? $($idents:ident : $types:ty),) -> $ret:ty $block:block $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(self, $($idents : $types),) -> $ret; $($rest)); };
	// (mut self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(mut self$(,)? $($idents:ident : $types:ty),) -> $ret:ty $block:block $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(mut self, $($idents : $types),) -> $ret; $($rest)); };
	// (&self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(&self$(,)? $($idents:ident : $types:ty),) -> $ret:ty $block:block $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(&self, $($idents : $types),) -> $ret; $($rest)); };
	// (&mut self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(&mut self$(,)? $($idents:ident : $types:ty),) -> $ret:ty $block:block $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(&mut self, $($idents : $types),) -> $ret; $($rest)); };


	// [No Block + No Ret] Add in return types if they're not specified:
	// (none)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident($($idents:ident : $types:ty),); $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name($($idents : $types),) -> (); $($rest)); };
	// (self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(self$(,)? $($idents:ident : $types:ty),); $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(self, $($idents : $types),) -> (); $($rest)); };
	// (mut self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(mut self$(,)? $($idents:ident : $types:ty),); $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(mut self, $($idents : $types),) -> (); $($rest)); };
	// (&self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(&self$(,)? $($idents:ident : $types:ty),); $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(&self, $($idents : $types),) -> (); $($rest)); };
	// (&mut self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(&mut self$(,)? $($idents:ident : $types:ty),); $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(&mut self, $($idents : $types),) -> (); $($rest)); };


	// [Block + No Ret] Strip blocks + add in return types:
	// (none)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident( $($idents:ident : $types:ty),) $block:block $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name($($idents : $types),) -> (); $($rest)); };
	// (self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(self$(,)? $($idents:ident : $types:ty),) $block:block $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(self, $($idents : $types),) -> (); $($rest)); };
	// (mut self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(mut self$(,)? $($idents:ident : $types:ty),) $block:block $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(mut self, $($idents : $types),) -> (); $($rest)); };
	// (&self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(&self$(,)? $($idents:ident : $types:ty),) $block:block $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(&self $($idents : $types),) -> (); $($rest)); };
	// (&mut self)
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, $(#[$attr:meta])* fn $fn_name:ident(&mut self$(,)? $($idents:ident : $types:ty),) $block:block $($rest:tt)) => {
	$crate::func_sig!($(+($indir))? $(%($i_im, $i_mut))? $($nom)?, fn $fn_name(&mut self, $($idents : $types),) -> (); $($rest)); };


	// And, finally, the end:
	($(+($indir:tt))? $(%($i_im:ident, $i_mut:ident))? $($nom:ident)?, ) => {};
	}

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Drop the `peripheral_trait!` macro #179

what

Background

Problems

A Solution

`Peripherals` trait

`PeripheralsWrapper`

steps

where

open questions

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Uh oh!

Drop the peripheral_trait! macro #179

Description

what

Background

Problems

A Solution

Peripherals trait

PeripheralsWrapper

steps

where

open questions

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions

Drop the `peripheral_trait!` macro #179

`Peripherals` trait

`PeripheralsWrapper`