Typenum list #232

Ben-PH · 2024-05-03T21:22:37Z

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 · 2024-05-03T21:24:22Z

core/src/hlist.rs

+                // this is how it's done at the type-level
+                // <<<Self as HList>::Len as typenum::IsEqual<typenum::U0>>::Output as typenum::Bit>::BOOL


I've left this in to illustrate that this change doesn't forec the new way of doing things. Things can get pretty gnarly.

Ben-PH · 2024-05-03T21:27:01Z

core/src/hlist.rs

+    <<T1 as HZippable<T2>>::Zipped as HList>::Len: Add<UInt<UTerm, B1>>,
+    <<<T1 as HZippable<T2>>::Zipped as HList>::Len as Add<UInt<UTerm, B1>>>::Output: Unsigned,


a) we need to pull out the type-defined length of the post-zipped list, and ensure that it is a type that implements the Add trait, effectively "the type cat play the role of a in a + 1"
b) with that addition constaint satisfied, we need to get the output, and ensure that it is a type that can fill the roll of b in a + 1 = b

Ben-PH · 2024-05-03T21:28:48Z

core/src/hlist.rs

-        let mut v = Vec::with_capacity(<Self as HList>::LEN);
+        let mut v = Vec::with_capacity(<<Self as HList>::Len as Unsigned>::USIZE);


One option is to return an array instead of a vector, because now the length is fundementally know at compile time. GenericArray could be used, making this a no_std compatable conversion

lloydmeta

Overall, I understand the motivation for this. At the same time, I'm weighing if this should be in frunk , and thus force a dependency on typenum and associated compile-time-hit, or whether users who need this should just enable nightly and use the generic_const_exprs option (playground example).

lloydmeta · 2024-05-04T04:42:30Z

core/src/hlist.rs

@@ -93,9 +102,10 @@ pub trait HList: Sized {
    /// assert_eq!(h.len(), 2);
    /// # }
    /// ```
+    #[deprecated(since = "0.5.0", note = "Please use <Self as HList>::Len::[USIZE | U8 | U32 | ... ] instead")]


I think it would not make much sense to deprecate this method and push users towards using the Typenum-coupled type.

lloydmeta · 2024-05-04T04:42:47Z

core/src/hlist.rs

@@ -127,7 +137,7 @@ pub trait HList: Sized {
    /// assert_eq!(<HList![i32, bool, f32]>::static_len(), 3);
    /// # }
    /// ```
-    #[deprecated(since = "0.1.31", note = "Please use LEN instead")]
+    #[deprecated(since = "0.1.31", note = "Please use Len::USIZE instead")]


On the other hand, I think we should probably think about finally removing this long-deprecated method..

Ben-PH · 2024-05-04T07:42:00Z

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 · 2024-05-04T11:45:55Z

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 · 2024-05-04T16:07:54Z

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 · 2024-05-04T20:45:06Z

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.

lloydmeta · 2024-05-05T04:03:22Z

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

Ben-PH · 2024-05-05T12:14:40Z

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:

make frunk nightly, just for the doc-generation stuff.
don't include the typenum feature flag in the published documentation, hiding it from view
"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"
manually annotate each feature-gated aspect of the library.
keep trying to make these cfg_attr hacks, and similar things work.

of these, I'm leaning towards 3.

ewww.
This is not doing the right thing by the community. Documentation is sacred
Incurs a minor pain-point, with immediate feedback on resolution
This would absorb work from more useful endevours
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 · 2024-05-06T07:20:35Z

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 · 2024-05-06T11:09:29Z

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 · 2024-05-06T11:41:42Z

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 · 2024-05-06T11:43:42Z

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "frunk"
 edition = "2021"
-version = "0.4.2"
+version = "0.5.0"


Does this version bumb still need to happen? except for the removal of a long-deprecated function, all changes are backwards compatable and additions are feature-gated. perhaps a bump to 0.4.3 instead?

...then again, pre-major reseales, bumps are pretty much free anyway...

Ben-PH · 2024-05-06T11:45:45Z

core/src/hlist.rs

+#[cfg(feature = "typenum")]
+impl<H, T: HList> HList for HCons<H, T>
+where
+    <T as HList>::Len: Add<B1>,
+    <<T as HList>::Len as Add<B1>>::Output: Unsigned,
+{
+    type Len = <<T as HList>::Len as Add<B1>>::Output;
+    const LEN: usize = 1 + <T as HList>::LEN;
+}
+#[cfg(not(feature = "typenum"))]
 impl<H, T: HList> HList for HCons<H, T> {
    const LEN: usize = 1 + <T as HList>::LEN;


Any suggestions RE: this duplication? it's exactly the same except for the added where-clases, but if there's a way to feature gate where-clases only, I'm unfamiliar

Ben-PH · 2024-05-06T11:49:17Z

core/src/hlist.rs

@@ -1433,6 +1486,26 @@ where
    }
 }

+#[cfg(feature = "typenum")]


A bit out of scope for this PR, but I feel this should return an array of known-length, and it be up to the library user to say "no, I want it as a Vec".

Perhaps a middle ground though: implement the into array, and into vec is provided as a conveniece wrapper arround let ??? = list.into(); let vector = ???.into();

Ben-PH · 2024-05-06T11:50:13Z

core/src/hlist.rs

+        #[cfg(feature = "typenum")]
+        assert_eq!(<HList![usize, &str, f32] as HList>::Len::USIZE, 3);


More testing needed?

Ben-PH · 2024-05-06T15:59:19Z

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

Ben-PH · 2024-05-06T18:30:50Z

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

Ben-PH added 4 commits May 3, 2024 22:16

const LEN -> type LEN

10960f6

deprecate len() for Self::Len + version-bump

5678174

Correct doc-test.

001a667

add "teaser-trailer" of using typnum internally

bdad2d1

Ben-PH commented May 3, 2024

View reviewed changes

lloydmeta reviewed May 4, 2024

View reviewed changes

cargo fmt

a024384

Ben-PH added 3 commits May 4, 2024 14:06

update length-get support

f2ff09c

retain the const-generic

91f4442

Refine doc-comments.

e0783fc

Ben-PH added 12 commits May 4, 2024 19:04

Clean some things up

b1521ae

cargo fmt

053516a

fix obo error

327efd1

Use Self::LEN internally

e3995ca

hide the use of typenum behind feature gate

cb668a9

In root, cargo test --all pass with/without typenum

56484d5

cargo fmt

8f83135

Render feature-gate in built docs

8d687fe

modify doc-publish to build (just docs) with nightly

6a86de8

include typenum feature in ci doc-check

02bbdf8

nightly features only used with -F nightly

8a9775a

cargo fmt --all

f39266f

Ben-PH added 2 commits May 6, 2024 13:05

Revert nightly stuff. Let error+suggestions help.

ce74350

Simplify typnum inclusion

906c499

Ben-PH added 2 commits May 6, 2024 13:28

Restore the core no_std

7cb7052

refine doc-test

fb310a6

Ben-PH commented May 6, 2024

View reviewed changes

Lift the typenum feature to root toml

424d769

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typenum list #232

Typenum list #232

Ben-PH commented May 3, 2024

Ben-PH May 3, 2024

Ben-PH May 3, 2024

Ben-PH May 3, 2024

lloydmeta left a comment

lloydmeta May 4, 2024

lloydmeta May 4, 2024

Ben-PH commented May 4, 2024

lloydmeta commented May 4, 2024

Ben-PH commented May 4, 2024

Ben-PH commented May 4, 2024 •

edited

Loading

lloydmeta commented May 5, 2024

Ben-PH commented May 5, 2024

lloydmeta commented May 6, 2024

Ben-PH commented May 6, 2024

Ben-PH commented May 6, 2024

Ben-PH May 6, 2024

Ben-PH May 6, 2024

Ben-PH May 6, 2024

Ben-PH May 6, 2024

Ben-PH commented May 6, 2024

Ben-PH commented May 6, 2024

		// this is how it's done at the type-level
		// <<<Self as HList>::Len as typenum::IsEqual<typenum::U0>>::Output as typenum::Bit>::BOOL

		<<T1 as HZippable<T2>>::Zipped as HList>::Len: Add<UInt<UTerm, B1>>,
		<<<T1 as HZippable<T2>>::Zipped as HList>::Len as Add<UInt<UTerm, B1>>>::Output: Unsigned,

		let mut v = Vec::with_capacity(<Self as HList>::LEN);
		let mut v = Vec::with_capacity(<<Self as HList>::Len as Unsigned>::USIZE);

		#[cfg(feature = "typenum")]
		assert_eq!(<HList![usize, &str, f32] as HList>::Len::USIZE, 3);

Typenum list #232

Are you sure you want to change the base?

Typenum list #232

Conversation

Ben-PH commented May 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lloydmeta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ben-PH commented May 4, 2024

lloydmeta commented May 4, 2024

Ben-PH commented May 4, 2024

Ben-PH commented May 4, 2024 • edited Loading

lloydmeta commented May 5, 2024

Ben-PH commented May 5, 2024

lloydmeta commented May 6, 2024

Ben-PH commented May 6, 2024

Ben-PH commented May 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ben-PH commented May 6, 2024

Ben-PH commented May 6, 2024

Ben-PH commented May 4, 2024 •

edited

Loading