frunk icon indicating copy to clipboard operation
frunk copied to clipboard

Typenum list

Open Ben-PH opened this issue 1 year ago • 11 comments

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 avatar May 03 '24 21:05 Ben-PH

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.

Ben-PH avatar May 04 '24 07:05 Ben-PH

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>;
}

lloydmeta avatar May 04 '24 11:05 lloydmeta

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 avatar May 04 '24 16:05 Ben-PH

I've updated with the following changes:

  • removal of the deprecated const_len
  • un-deprecation of the len function
  • un-removal of const Self::LEN
  • putting typenum and its use behind a feature flag
  • documentation including the typenum feature in CI
  • items gated behind typenum and std features in hlist.rs are indicated as such in the documentation rendering.*

...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.

Ben-PH avatar May 04 '24 20:05 Ben-PH

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

lloydmeta avatar May 05 '24 04:05 lloydmeta

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...

Ben-PH avatar May 05 '24 12:05 Ben-PH

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.

lloydmeta avatar May 06 '24 07:05 lloydmeta

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 avatar May 06 '24 11:05 Ben-PH

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 avatar May 06 '24 11:05 Ben-PH

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

Ben-PH avatar May 06 '24 15:05 Ben-PH

....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<Len = DC::Len>> { | ^^^^^^^^^^^^^^^^^^^^ 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 <PinsT as HList>::Len: Add<B1> {

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?

Ben-PH avatar May 06 '24 18:05 Ben-PH