Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Struct construction using an hlist #222

Open
Ben-PH opened this issue Aug 27, 2023 · 12 comments
Open

Struct construction using an hlist #222

Ben-PH opened this issue Aug 27, 2023 · 12 comments

Comments

@Ben-PH
Copy link

Ben-PH commented Aug 27, 2023

In order to create a USB device in the esp32s3-hal crate, you must provide it with the correct pins, which is constrained by marker traits. Current way of doing things, is select the correct pins in the Pins struct, then invoke the USB::new() constructor, like so.

I'm working on managing pins as an HList - you start with all the pins, and as you use them to make peripherals, they move from that list, into the device struct at the type-level: esp-rs/esp-hal#748

For structs that need more than one pin, the solution from what I've worked out is an esoteric where clause. If the burden of setting up this where clause is on the user, that would make the hal-crate pretty much unusable:

here is a working example. That `where` clause is _very_ non-trivial if you don't already know what to do:
struct Blinker {
    pin4: GpioPin<Output<PushPull>, 4>,
    pin5: GpioPin<Output<PushPull>, 5>,
}

impl Blinker {
    fn initialize<L0, L0Remaining, L1Remaining>(
        io: IO<L0>,
    ) -> (
        Self,
        IO<<L0::Remainder as Plucker<GpioPin<Unknown, 5>, L1Remaining>>::Remainder>,
    )
    where
        L0: Plucker<GpioPin<Unknown, 4>, L0Remaining>,
        <L0 as Plucker<GpioPin<Unknown, 4>, L0Remaining>>::Remainder:
            Plucker<GpioPin<Unknown, 5>, L1Remaining>,
    {
        let (pin4, io) = io.pluck_pin();
        let mut pin4 = pin4.into_push_pull_output();
        pin4.set_high().unwrap();

        let (pin5, io) = io.pluck_pin();
        let mut pin5 = pin5.into_push_pull_output();
        pin5.set_high().unwrap();
        (Self { pin4, pin5 }, io)
    }

One thing that can simplify things:

Multi-pluck constructor via the builder: The complexity is reduced, but watch the verbosity...
impl<T> BlinkerBuilder<T> {
    fn new(io: IO<T>) -> Self {
        Self {
            io,
            pin1: None,
            pin2: None,
        }
    }
    fn set_pin1<Remain>(self) -> BlinkerBuilder<T::Remainder>
    where
        T: Plucker<GpioPin<Unknown, 4>, Remain>,
    {
        let (pin, io) = self.io.pluck_pin();
        let pin = pin.into_push_pull_output();
        BlinkerBuilder {
            io,
            pin1: Some(pin),
            pin2: self.pin2,
        }
    }

    fn set_pin2<Remain>(self) -> BlinkerBuilder<T::Remainder>
    where
        T: Plucker<GpioPin<Unknown, 5>, Remain>,
    {
        let (pin, io) = self.io.pluck_pin();
        let pin = pin.into_push_pull_output();
        BlinkerBuilder {
            io,
            pin1: self.pin1,
            pin2: Some(pin),
        }
    }
    fn build(self) -> (Blinker, IO<T>) {
        let bl = Blinker {
            pin4: self.pin1.unwrap(),
            pin5: self.pin2.unwrap(),
        };
        (bl, self.io)
    }
}
For me, the dealbreaker is it forces the user into a very specific code pattern. The goal is constrain-to-correctness, and resolve ownership issues, at the type-level. Forcing a pattern is an anti-goal. It would also prevent construction-from-hlist overly burdensome to implement/maintain in the esp-hal crate itself.

Would it be useful to write a proc-macro that could be used like so?

// expands into something like the builder pattern I showed earlier
#[frunk::hl_builder]
struct Blinker {
    // So the proc macro knows to pluck the value from the hlist being used
    // probably not good to have the init option to the annotion
    // Instead, blinker should just call the construction
    // in its `initialize`/`new` method, and run the per-pin inits itself...
    #[pluck_build(init = pin4_init)]
    pin4: GpioPin<Output<PushPull>, 4>,
    #[pluck_build(init = pin5_init)]
    pin5: GpioPin<Output<PushPull>, 5>,
}

I also recognize that this is getting a bit crazy. Is it possible that I'm missing something that would simplify things? is there already something baked into this crate that constructs by moving the types from a typelist into a struct? That's essentially what I'm trying to do. Generics seem to be more about transforming between structs that only differ in name. That's close, but I need something that differ by a const-generic value only. Same name, otherwise.

@Ben-PH
Copy link
Author

Ben-PH commented Aug 28, 2023

I put this together: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=0a4172aa0c5df1ca06113d5fa18cb279

This is the first stages of a macro:

use frunk::hlist;

#[frunk::hl_build]
struct ListConstructed {
    #[hl_field]
    field1: u8,
    #[hl_field]
    field2: String,
    user_defined: i32,

}

fn foo() {
    let list = hlist!(3u8, true, String::from("list-str"), 10.4);
    let (builder, new_list) = ListConstructed::hl_new(list, -42);
    assert_eq!(new_list, hlist!(true, 10.4));
}

...the proc-macro would add an impl with:

fn hl_new<L0, L1, L2>(L0, user_defined: i32) -> (Self, <<L0 as ...., L2>>::Remainder)
where
    L0: Plucker<u8, L1>,
    <L0 as Plucker<u8, L1>>::Plucker: Plucker<String, L2>
{
    // pluck field 1 and 2 from the list to construct
}

I'll get to work on putting a proc-macro together based on this.

@Ben-PH
Copy link
Author

Ben-PH commented Aug 28, 2023

Progress report:

use frunk::{hlist::Plucker, HList};

#[derive(Debug)]
#[hl_build_macro::hl_build]
pub struct ReferenceStruct {
    #[hl_field]
    field0: u8,
    #[hl_field]
    field1: bool,
    field2: f32,
}

pub fn demo_use() {
    let list = frunk::hlist!(true, 3u8, String::from("list-str"), 10.4);
    let (blinker, list): (_, HList!(String, f32)) = ReferenceStruct::hl_new(list, 69.420);
    println!("{:?}", blinker);
    println!("{:?}", list);
}

will print out:

ReferenceStruct { field0: 3, field1: true, field2: 69.42 }
HCons { head: "list-str", tail: HCons { head: 10.4, tail: HNil } }

...the result of cargo expand:

pub struct ReferenceStruct {
    field0: u8,
    field1: bool,
    field2: f32,
}
impl ReferenceStruct {
    fn hl_new<L0, L1, L2>(
        l0: L0,
        field2: f32,
    ) -> (Self, <<L0 as Plucker<u8, L1>>::Remainder as Plucker<bool, L2>>::Remainder)
    where
        L0: Plucker<u8, L1>,
        <L0 as Plucker<u8, L1>>::Remainder: Plucker<bool, L2>,
    {
        let (field0, l1) = l0.pluck();
        let (field1, l2) = l1.pluck();
        return (Self { field0, field1, field2 }, l2);
    }
}

...add a couple more fields, and it still works nicely:

pub struct ReferenceStruct {
    field0: u8,
    field1: bool,
    field2: f32,
    fielda: u16,
    fieldb: i16,
}
impl ReferenceStruct {
    fn hl_new<L0, L1, L2, L3, L4>(
        l0: L0,
        field2: f32,
    ) -> (
        Self,
        <<<<L0 as Plucker<
            u8,
            L1,
        >>::Remainder as Plucker<
            bool,
            L2,
        >>::Remainder as Plucker<u16, L3>>::Remainder as Plucker<i16, L4>>::Remainder,
    )
    where
        L0: Plucker<u8, L1>,
        <L0 as Plucker<u8, L1>>::Remainder: Plucker<bool, L2>,
        <<L0 as Plucker<
            u8,
            L1,
        >>::Remainder as Plucker<bool, L2>>::Remainder: Plucker<u16, L3>,
        <<<L0 as Plucker<
            u8,
            L1,
        >>::Remainder as Plucker<
            bool,
            L2,
        >>::Remainder as Plucker<u16, L3>>::Remainder: Plucker<i16, L4>,
    {
        let (field0, l1) = l0.pluck();
        let (field1, l2) = l1.pluck();
        let (fielda, l3) = l2.pluck();
        let (fieldb, l4) = l3.pluck();
        return (
            Self {
                field0,
                field1,
                fielda,
                fieldb,
                field2,
            },
            l4,
        );
    }
}

@lloydmeta I would like to make a PR. I'm thinking of putting it behind a feature gate "hlist_construction": thoughts/suggestions/comments?

improvements:

  • allow initialization as part of each pluck: you can pluck a value, but then you might want to configure/sanity-check said value. would be nice to include that somehow.
  • to handle plucking by constraints: constrain in the where clause (or wherever) such that you can pluck-by-type-constraints (e.g. if only one type in the list impls Foo trait, pluck that one)
  • ???

@lloydmeta
Copy link
Owner

Thanks @Ben-PH for your issue + the work you've put into this. I can certainly see that you have a use case for this.

I'm currently on vacation, so please for give the "drive by" nature of this comment; I may have missed a thing or two.

In the spirit of trying to build on already-existing things, I'm curious if you've considered using something like from_generic to do something like this?

fn test_struct_from_generic() {
let h = hlist!("Humpty", "Drumpty", 3);
let p: Person = from_generic(h);
assert_eq!(
p,
Person {
first_name: "Humpty",
last_name: "Drumpty",
age: 3,
}
);
}

There's also the ability to do something similar using LabelledGeneric, though I'm unsure/don't think it's what you're after

fn test_struct_from_labelled_generic() {
let h = hlist![
field!((f, i, r, s, t, __, n, a, m, e), "Humpty"),
field!((l, a, s, t, __, n, a, m, e), "Drumpty"),
field!((a, g, e), 3),
];
let u: NewUser = from_labelled_generic(h);
assert_eq!(
u,
NewUser {
first_name: "Humpty",
last_name: "Drumpty",
age: 3,
}
);
}

@Ben-PH
Copy link
Author

Ben-PH commented Aug 29, 2023

I'm trying to encapsulate behavior specification at the type-level. In my specific case, we have zero-sized-types, and what can be done is defined by the impls on these types. From my glance at generics, it's more about "To construct a struct, first construct an hlist of values that is compatable with the struct, and move them all into the struct"

I'm more looking for: "You have a list of singletons. move those singletons into a struct to construct them. Do this all within the type-system"

I'm too rusty with actual FP to actually write it out in acurate FP syntax, but it's more like your typical haskell state-machine, but strictly at the type level

makeThing :: (TypeList a, TypeList b, Thing t) :: a => (t, b)

they are different type-lists, because a (bool, (String, ())) is a different type to (bool, ()), which illustrates the sort of change you might see in TypeList between a and b.

Also: I got nerd-sniped, and wanted to see if I could / have some practice at this sort of work. I won't be dissapointed if my PR ends up getting rejected; my goal is to have this particular functionality available to the dependant work (esp32-hal crate). If this is extending existing features, or just learning that my work is redundant to already existing features, that's fine, or even something independent of frunk, I'll be happy :)

@lloydmeta
Copy link
Owner

I'm almost certain that there is something that I'm missing :)

I'm more looking for: "You have a list of singletons. move those singletons into a struct to construct them. Do this all within the type-system"

^ can you please give an example of this in code? It's difficult for me to understand the difference between what (Labelled)Generic can do (e.g. through transmog or from_generic/from_labelled_generic) versus what it is you're trying to have the type system do for you. Apologies if I'm being slow here :)

@Ben-PH
Copy link
Author

Ben-PH commented Aug 31, 2023

Sure thing. I'll put something together to illustrate the difference in more detail.

Apologies if I'm being slow here

It's on me. Perhaps the problem I'm solving is a bit more niche than I thought, or perhaps I got excited about a solution, and have a bit of selection bias happening on the value of the solution.

There's a community meeting with the esp-rs team today. I'll be talking a bit about it there. If you can't make it, I'll ask for notes and include it here so you can grab more context. Here is some announcement text:

The new meeting was scheduled: Rust ESP32 Community Meeting 2023-08-31
Thursday, 31 August · 17:00 – 18:00 CEST/GMT+2 (a.k.a. Brno time)
Google Meet joining info
Video call link: https://meet.google.com/nxo-hnhw-rsk
Submit topics for discussion: https://github.com/esp-rs/rust/discussions/189

I'll now be getting to work modifying my fork of esp-hal to include the content of this PR, as well as my microcontroller project. In the meantime, Here are some illustrative examples of

With the "pins-in-struct" pattern, making a 3-pin blinker peripheral looks like this:
struct Blinker {
    pin4: GpioPin<Output<PushPull>, 4>,
    pin5: GpioPin<Output<PushPull>, 5>,
    pin6: GpioPin<Output<PushPull>, 6>,
}

// then inside main:

    let io = IO::new(peripherals.GPIO, peripherals.IO_MUX);

    // have to move pins out of io to avoid partial moves
    let pin4 = io.pins.gpio4;
    let pin4 = pin4.into_push_pull_output();
    let pin5 = io.pins.gpio5;
    let pin5 = pin5.into_push_pull_output();
    let pin6 = io.pins.gpio6;
    let pin6 = pin6.into_push_pull_output();
    let blinker = Blinker {
        pin4,
        pin5,
        pin6,
    };

To use a dependency injection model, where you provide the collection of available resources externally, a struct won't do: you get partial move errors. A list is perfect: it's pretty much designed for partial-moves, but:

To make a list-based initializer, you need to hand-write theses generics:
struct Blinker {
    pin4: GpioPin<Output<PushPull>, 4>,
    pin5: GpioPin<Output<PushPull>, 5>,
    pin6: GpioPin<Output<PushPull>, 6>,
}


impl Blinker {
    fn initialize<L0, L1, L2, L3>(
        io: L0,
    ) -> (
        Self,
        <<<L0 as Plucker<GpioPin<esp32s3_hal::gpio::Unknown, 4>, L1>>::Remainder as Plucker<GpioPin<esp32s3_hal::gpio::Unknown, 5>, L2>>::Remainder as Plucker<GpioPin<esp32s3_hal::gpio::Unknown, 6>, L3>>::Remainder,
    )
    where
        L0: Plucker<GpioPin<Unknown, 4>, L1>,
        <L0 as Plucker<u8, L1>>::Remainder: Plucker<bool, L2>,
        <<L0 as Plucker<GpioPin<Unknown, 4>, L1>>::Remainder as Plucker<GpioPin<Unknown, 5>, L2>>::Remainder: Plucker<GpioPin<Unknown, 6>, L3>,
    {
        let (pin4, io) = io.pluck();
        let mut pin4 = pin4.into_push_pull_output();
        pin4.set_high().unwrap();

        let (pin5, io) = io.pluck();
        let mut pin5 = pin5.into_push_pull_output();
        pin5.set_high().unwrap();

        let (pin6, io) = io.pluck();
        let mut pin6 = pin6.into_push_pull_output();
        pin6.set_high().unwrap();
        (Self { pin4, pin5, pin6 }, io)
    }
}
This PR (plus a bit of work), list-dep-injected code is simple:
#[derive(frunk::ListBuild)]
struct Blinker {
    #[init(into_push_pull_output)]
    #[init(set_high)]
    #[pluck(GpioPin<Unknown, 4>)]
    pin4: GpioPin<Output<PushPull>, 4>,
    #[init(into_push_pull_output)]
    #[init(set_high)]
    #[pluck(GpioPin<Unknown, 5>)]
    pin5: GpioPin<Output<PushPull>, 5>,
    #[pluck(GpioPin<Unknown, 6>)]
    #[init(into_push_pull_output)]
    #[init(set_high)]
    pin6: GpioPin<Output<PushPull>, 6>,
}



fn main() -> ! {

    // snip 
    let io = IO::hl_new(peripherals.GPIO, peripherals.IO_MUX);
    let (blinker, _io) = Blinker::initialize(io.pins);
    // snip...
}

I still need to add the feature allowing for initialization, and I'm sure it will take a nicer form somehow, but the base principals are there.

I'll be putting together more e.g.s over the next few days/week or so.

@lloydmeta
Copy link
Owner

Thanks so much for taking the time and effort to put that together; I promise I'll read it and get back to you. Just a quick follow up question: In that last snippet,

let io = IO::hl_new(peripherals.GPIO, peripherals.IO_MUX);
let (blinker, _io) = Blinker::initialize(io.pins);

Is the io binding an HList? I think it is, but it's also got a pins method in line 2?

Full disclosure: back from vacation but wife caught covid so time is one again, limited 🤦🏼 .

@Ben-PH
Copy link
Author

Ben-PH commented Sep 2, 2023

ah yes, there is a bit of a mix up there. Inside the esp32 hal crate, the IO struct (input output) looks like this:

/// General Purpose Input/Output driver
pub struct IO {
    _io_mux: IO_MUX,
    pub pins: Pins,
}

I think your question comes from me oscillating between a pattern that serves as an illustrative example, and a pattern that I've been using to validate the hand-expanded version of the derive macro.

For the purpose of this context, you can consider io as an implementer of Plucker. To consider io an HList is close enough.

With that out of the way....

I plan on introducing changes to make pin field a T-generic, so that it can be a Pins or an HList (depending on whether you create the IO struct with ::new or ::hl_new). In order for that to work, the derive macro will have to have a way of recognizing that IO<T> is a wrapper around a T HList. This might, in turn, help if there was a #[derive(frunk::ListWrapper)]. Something like this:

/// General Purpose Input/Output driver
#[derive(frunk::ListWrapper)]
pub struct IO<T> {
    _io_mux: IO_MUX,
    #[list]
    pub pins: T,
}

// the derive would expand to something like this:
impl<L0, L1, Ty> Plucker<Ty, L0> for IO<L0 as Plucker<Ty, L1>::Remainder> 
where
    L0: Plucker<Ty, L1>
{
    fn pluck(in_list: Self) -> (Ty, IO<<L0 as Plucker<Ty, L1>::Remainder>) {
        todo!("pluck the value, and reconstruct a new IO<L1>")
    }
}

I appreciate your attention into this. In hindsight, I should have put my focus into other projects of mine given you are on holidays. I'll try to step back from calling on your attention for another week or two: work is a means to enrich life, and I would hate to contribute to those roles being inverted.

@lloydmeta
Copy link
Owner

Sorry, taking another peek at this. I'll try to ask my question in a way that tries to utilise existing Frunk functions as a way to try to understand it better 🙏🏼 Another goal is to avoid bespoke macros where possible.

Would something like this be a rough equivalent

  1. Running an existing hlist through map to do the transformation/initialisation on a per-element basis.
  2. Using from_generic to get to your target struct

    frunk/core/src/generic.rs

    Lines 123 to 129 in 83b26a1

    /// Given a generic representation `Repr` of a `Dst`, returns `Dst`.
    pub fn from_generic<Dst, Repr>(repr: Repr) -> Dst
    where
    Dst: Generic<Repr = Repr>,
    {
    <Dst as Generic>::from(repr)
    }

I think we might be missing something, like returning the unused remainder from (2), but that should be relatively simple to add support for.

@Ben-PH
Copy link
Author

Ben-PH commented Oct 5, 2023

On point 1:

Where iteration semantics are valuable, the HCons methods such as map are useful, but that's a niche case. The feature that I am ultimately hoping for is two-fold:

  • a means to write out a struct such that a constructor is generated. The constructor populates fields by plucking out entries from a passed in list.
  • the ability to delegate a field in a struct as "the hlist field". Where something needs a list, you can instead use this struct, and it will be known to the compiler that the field will be used for the list requirements. A bit like impl Deref, and used with the *& prefix.

If one of the fields in struct with a derived constructor with an hlist as a dependance, is itself an hlist, then I imagine some sort of iterative process to move the values from the provided hlist into the field of the object under construction. Ideally, there would be a means to process each individual value as well (in my specific use-case, it could include setting gpio pins to an initial state, but the per-entry initialization would need to be general in nature).

point being, is that iterative processes would be one element of the feature I envisage.

regarding point 2.

I think the ListWrapper struct idea is separate to the constructor derivation idea. Probably best we separate the discussions to avoid confusion.

@Ben-PH
Copy link
Author

Ben-PH commented Oct 5, 2023

to provide a real-world example of what I have in mind, consider this code:

    let usb = USB::new(
        peripherals.USB0,
        io.pins.gpio18,
        io.pins.gpio19,
        io.pins.gpio20,
        &mut system.peripheral_clock_control,
    );

This code could compiles fine, but if I change the pin-order, swapping 18 and 19, i get errors, one of which looks like this.

error[E0277]: the trait bound `GpioPin<esp32s3_hal::gpio::Unknown, 19>: UsbSel` is not satisfied
  --> core/src/main.rs:68:15
   |
68 |     let usb = USB::new(
   |               ^^^ the trait `UsbSel` is not implemented for `GpioPin<esp32s3_hal::gpio::Unknown, 19>`
   |
   = help: the trait `UsbSel` is implemented for `GpioPin<T, 18>`
note: required by a bound in `esp32s3_hal::otg_fs::USB`
  --> /home/ben/.cargo/registry/src/index.crates.io-6f17d22bba15001f/esp-hal-common-0.12.0/src/otg_fs.rs:50:8
   |
48 | pub struct USB<'d, S, P, M>
   |            --- required by a bound in this struct
49 | where
50 |     S: UsbSel + Send + Sync,
   |        ^^^^^^ required by this bound in `USB`

NOTE: GpioPin<..., 19> (i.e. io.pins.gpio19), does not satisfy the type-system constraints of USB::new(). If you were to examine the code base being used, you would see there is only one pin configuration that is valid, and this is enforced through the type-system. I happen to be compiling for an esp32s3 in this case, but the numbers can change, depending on which platform is used (never more than one choice when it comes to pin-selection).

...this would imply that the following code is possible:

    let (select, pin_list) = pin_list.pluck();
    let (data_pstv, pin_list) = pin_list.pluck();
    let (data_ngtv, pin_list) = pin_list.pluck();
    let usb = USB::new(
        peripherals.USB0,
        select,
        data_pstv,
        data_ngtv,
        &mut system.peripheral_clock_control,
    );

This code is now valid for any platform that upholds the "only one valid type on the pin-args" contract, making it platform agnostic. What I want to do, is take it a step further:

    let (pin_list, usb) = USB::hlist_new(
        peripherals.USB0,
        pin_list,
        &mut system.peripheral_clock_control,
    );

There are other benefits relating to ownership rules that come from being able to use hlists in this manner, but I feel it's out of scope of this discussion (for now)

@lloydmeta
Copy link
Owner

Thanks @Ben-PH . I've given this some more thought, and my current feeling is that the exact solution you're after here is a tad niche for inclusion into frunk, which tries to be general, with tools that compose of small, simple building blocks.

I'd like to suggest that you implement the macro in your library for now, and if there is more demand + usage from others, we can definitely re-open the discussion include it, or some form of it, in frunk 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants