lloydmeta / frunk

Funktional generic type-level programming in Rust: HList, Coproduct, Generic, LabelledGeneric, Validated, Monoid and friends.
https://beachape.com/frunk/
MIT License
1.24k stars 56 forks source link

Typenum list #232

Open Ben-PH opened 4 months ago

Ben-PH commented 4 months ago

This creates a version bump and upgrades the HList assossciated assosciated const LEN to an assostiated type: typenum::Unsigned

Motivation is as follows: I have a trait that needs a fixed length heterogenous list, but the length is generic to only a few numbers. the non generic way of implementing it:

pub trait HWApi1 {
    fn config<S: SetDutyCycle>(freq: embedded_time::rate::Hertz<u32>, pinA: S);
}
pub trait HWApi2 {
    fn config<S: SetDutyCycle, S: SetDutyCycle>(
        freq: embedded_time::rate::Hertz<u32>,
        pinA: S,
        pinB: S,
    );
}
pub trait HWApi3 {
    fn config<S: SetDutyCycle, S: SetDutyCycle, S: SetDutyCycle>(
        freq: embedded_time::rate::Hertz<u32>,
        pinA: S,
        pinB: S,
        pinC: S,
    );
}

I the requirements for this library are for HWApi[1, 2, 3, 4, 6] and I'd like to be able to define it as so:

pub trait HWApi<N>
where
    // constrain the values here
{
    fn config<L>(freq: ..., L) 
    where
        L: Hlist,
        <L as HList>::Len: IsEqual<N>,
        // make sure every item in the list is constrained to implementing `SetDutyCycle` here
    {

...This is only possible at the type level when HList has a canonical expression of its length at the type level as well.

Ben-PH commented 4 months ago

The problem arises when you want to apply constraints such as length. This fix addresses a limitation of compile-time constant generics, but doesn't fully leverage the type system.

For example, what if I want to constrain to a particular length? or that any given trait implementation, two values assosciated with a trait must satisfy some sort of truth? (a == b/2, or a < b). I'm not aware of any features, nightly or otherwise, that says you can do something like this:


trait SomeTrait<const LHS: usize, const RHS: u32> 
where
    LHS: < RHS,
    LHS + RHS: 10,
{

Const generics are a fine incremental improvement to the language, I have a strong opinion that it is no substitute for true compile-time, type-system-only, types-as-values utility.

lloydmeta commented 4 months ago

The problem arises when you want to apply constraints such as length. This fix addresses a limitation of compile-time constant generics, but doesn't fully leverage the type system.

I understand. My proposal was that const SIZE: usize can be used by lib users with typenum, e.g. in this Playground, by passing it to typenum.

Pasted here:

#![feature(generic_const_exprs)]

use typenum::*; // 1.17.0

trait HList {
    const SIZE: usize;
}

trait HWApi<N> {
    fn config<L>(hlist: L) -> ()
    where
        L: HList,
        Const<{ <L as HList>::SIZE }>: IsEqual<N>;
}
Ben-PH commented 4 months ago

I wasn't familiar with that as an option. I can see it coming in handy, but I still hold the view that there is merit to type-level expression of length being a core-feature of an HList construct.

An HList is fundamentally a type-construction, and that construction includes the type-construction of its length. In a pseudo-haskell way, look at how to use an HList like type-constructor but just to express numbers:

data List = Base | Node () List a list, but all types are (). analogous to a typed-num

vs not restricting to the () type only:

data List H = Base | Node H List a list analogous to an HList

It makes sense to me to retain the number-component of an HList type-construction. I can also see it making sense to keep the Self::LEN interface: at the end of the day, rust is primarily an imperitive language, and it's what people are used to.

The value of adding type Self::Len: Unsized; is that instead of needing to "implement after the fact" the means of recovering the length-component of the HList type-construction after throwing it away, it instead retains the complete representation of the type, and thus allowing the user to be free to "downgrade" to a value-representation in the form of their choosing, whether it be a Self::LEN to get the usize, <Self as HList>::Len::U8 in byte form, or to go straight into using the length as a type without having to reconstruct it from a value representation using the Const struct

Also, there might be corner cases where the utility of the type-len is diminished by relying on Const, but I can't think of anything specific.

As for the issue with typenum being an additional dependency, I'll have a look at making it a feature-gated dependency. Without that, how do you feel about re-exporting the typenum crate alongside HList?

Next update of this PR will include restoration of the const LEN: usize;

Ben-PH commented 4 months ago

I've updated with the following changes:

...that last point is a bit hacky. It needs a nightly feature to do it, but I don't want to turn this into a crate that needs a nightly build, especially if it's just for documentation. To this end, I added a nightly feature, and anything that needs nightly is gated behand that: cargo +stable build -F nightly will fail, but cargo doc --open -F typenum won't bring in the nightly features. The drawback is that the typenum features and re-expots will be documented, but it won't show up as feature gated.

On my CI, I get a failure when building for thumbv6m-none-eabi: cargo check --target thumbv6m-none-eabi --no-default-features

I get the same locally on my machine. Not exactly sure what's going on there. Will investigate and come back with findings.

lloydmeta commented 4 months ago

++ I like the new direction (typenum as a feature, restoring "normal" value based len).

Ben-PH commented 4 months ago

Big issues with the documentation. For me, I don't like the idea of docs.rs/frunk documenting the re-export of typenum or use of Self::Len without being very explicit that it's only enabled by feature-gate, but that requires nightly, plus the activation of docs_cfg nightly feature.

This makes things a bit silly in CI, CD, the use of cfg_attr etc. I really don't like it. Here are the options as I see it:

  1. make frunk nightly, just for the doc-generation stuff.
  2. don't include the typenum feature flag in the published documentation, hiding it from view
  3. "lie" about typenum stuff behind a non-default feature in published documentation, relying on cargo to warn "you need to activate the typenum feature to use this"
  4. manually annotate each feature-gated aspect of the library.
  5. keep trying to make these cfg_attr hacks, and similar things work.

of these, I'm leaning towards 3.

  1. ewww.
  2. This is not doing the right thing by the community. Documentation is sacred
  3. Incurs a minor pain-point, with immediate feedback on resolution
  4. This would absorb work from more useful endevours
  5. That is just being unkind to whoever needs to deal with it.

I also think 3 has a non-zero chance of smooth the way to resolving the CI issue.

This CI fail also merits a discussion-issue regarding the std feature...

lloydmeta commented 4 months ago

3. "lie" about typenum stuff behind a non-default feature in published documentation, relying on cargo to warn "you need to activate the typenum feature to use this"

I'm a bit confused about what you mean by "lie". To be clear are you saying that the compiler will error out if typenum is used w/o actually using the feature (as opposed to "warn", and succeed), telling the user to turn on the feature to resolve?

If so, I think that is fine. I'm -1 on compiler simply warning and succeeding compilation.

Ben-PH commented 4 months ago

To be clear are you saying that the compiler will error out [...]?

You are correct. I wrote "warn" when I meant "error".

I'll go with opt 3

Ben-PH commented 4 months ago

CI fixed on my end, and added typenum feature to the published docs.

My main concern now is that with the typenum length, HList implementations need to add where-clauses that rely on that feature. As far as I know, you can't hide the where-clause behind a feature gate, and so duplicate implementations must be written, using #[cfg[(not](.... to choose between the two. this is not all that nice, and welcome suggestions to improve, but as it stands, it's a required compromise if we want to put the typed-length behind a feature gate.

With that in mind, it might be worth adding some things to the CI work flow to run tests with/without the typenum feature, but I don't want to hack away at that too much, at least without your direction.

Ben-PH commented 4 months ago

...hold off on morging for a bit. I'm getting a feel for using it in practice.

Ben-PH commented 4 months ago

....there's a problem:

impl<DCT: HList, P: SetDutyCycle, PinsT: HList> PWMPinSetter<HCons<u16, DCT>, HCons<P, PinsT>> {
    fn set_pin(dc_list: HCons<u16, DCT>, pins: HCons<P, PinsT>) {
        pins.head.set_duty_pct(dc_list.head);
        PWMPinSetter::set_pin(dc_list.tail, pins.tail)
    }
}

...that's pretty damn close to what I want, but I get this error:

error[E0277]: cannot add B1 to <PinsT as HList>::Len --> src/hw_drivers.rs:15:49 15 impl<DCT: HList, P: SetDutyCycle, PinsT: HList> PWMPinSetter<HCons<u16, DCT>, HCons<P, PinsT>> { ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no implementation for <PinsT as HList>::Len + B1
= help: the trait Add<B1> is not implemented for <PinsT as HList>::Len, which is required by HCons<P, PinsT>: HList = note: required for HCons<P, PinsT> to implement HList note: required by a bound in PWMPinSetter --> src/hw_drivers.rs:10:38 10 struct PWMPinSetter<DC: HList, Pins: HList> { ^^^^^^^^^^^^^^^^^^^^ required by this bound in PWMPinSetter help: consider further restricting the associated type
15 impl<DCT: HList, P: SetDutyCycle, PinsT: HList> PWMPinSetter<HCons<u16, DCT>, HCons<P, PinsT>> where ::Len: Add {

basically, the way I enterpret it, is that each time you prepend a new head to an existing list, I've set it up so that type Len = Add1<<Tail as HList>::Len>:, which means that the tail must implement Add<B1>, and I haven't quite worked it out so that the library user doesn't have to provide the constraint.

My stratagy was to set it such that HList::Len: Unsigned + Add<B1>, and for struct HCons<H, T: HList>, which makes sense to me. clearing up the errors so that everywhere there is a tail, to add the HList constraint, but that started getting a bit painful in the gen_inherent_methods macro, and in other modules as well.

thoughts?