Introduce Transmogrifying from A to B #124

lloydmeta · 2018-09-24T13:08:08Z

This PR adds the ability to do "transmogrifying" (inspired by this Haskell repo and this Scala SO answer) to Frunk.

What is "transmogrifying"? In this context, it means to convert some data of type A into data of type B, in a typesafe, recursive way, as long as A and B are "similarly-shaped". In other words, as long as B's fields and their subfields are subsets of A's fields and their respective subfields, then A can be turned into B.

As usual, the goal with Frunk is to do this:

Using stable (so no specialisation, which would have been helpful, methinks)
Typesafe
No usage of unsafe

I think the compiler should be able to compile away most, if not all, of the performance penalties this imposes, much like it does for non-recursive "sculpting", but benchmarks will follow in another PR.

In any case, here is an example:

use frunk::labelled::Transmogrifier;
#[derive(LabelledGeneric)]
struct InternalPhoneNumber {
    emergency: Option<usize>,
    main: usize,
    secondary: Option<usize>,
}

#[derive(LabelledGeneric)]
struct InternalAddress<'a> {
    is_whitelisted: bool,
    name: &'a str,
    phone: InternalPhoneNumber,
}

#[derive(LabelledGeneric)]
struct InternalUser<'a> {
    name: &'a str,
    age: usize,
    address: InternalAddress<'a>,
    is_banned: bool,
}

#[derive(LabelledGeneric, PartialEq, Debug)]
struct ExternalPhoneNumber {
    main: usize,
}

#[derive(LabelledGeneric, PartialEq, Debug)]
struct ExternalAddress<'a> {
    name: &'a str,
    phone: ExternalPhoneNumber,
}

#[derive(LabelledGeneric, PartialEq, Debug)]
struct ExternalUser<'a> {
    age: usize,
    address: ExternalAddress<'a>,
    name: &'a str,
}

let internal_user = InternalUser {
    name: "John",
    age: 10,
    address: InternalAddress {
        is_whitelisted: true,
        name: "somewhere out there",
        phone: InternalPhoneNumber {
            main: 1234,
            secondary: None,
            emergency: Some(5678),
        },
    },
    is_banned: true,
};

/// Boilerplate-free conversion of a top-level InternalUser into an
/// ExternalUser, taking care of subfield conversions as well.
let external_user: ExternalUser = internal_user.transmogrify();

println!("{:?}", external_user);

let expected_external_user = ExternalUser {
    name: "John",
    age: 10,
    address: ExternalAddress {
        name: "somewhere out there",
        phone: ExternalPhoneNumber {
            main: 1234,
        },
    }
};

assert_eq!(external_user, expected_external_user);

ExpHP · 2018-09-24T14:08:16Z

core/src/labelled.rs

+                field!(is_admin, true),
+                field!(name, hlist![field!(is_admin, true)]),
+            ]
+        )


Because a lot of work is put into making identity transforms work, I'd like to see tests for:

Identity transform with labeled fields

type Source = Hlist![ Field<name, Hlist![ Field<inner, f32>, Field<is_admin, bool>, ]>, Field<age, i32>, Field<is_admin, bool>]; type Target = Source;

Non-identity transform where a labelled field gets an identity transform

type Source = Hlist![ Field<name, Hlist![ Field<inner, f32>, Field<is_admin, bool>, ]>, Field<age, i32>, Field<is_admin, bool>]; type Source = Hlist![ Field<age, i32>, Field<name, Hlist![ Field<inner, f32>, Field<is_admin, bool>, ]>];

Looks like GH ate my comment; I gave these a go but they currently don't compile, and I suspect it's due to ambiguous identity impls for the inner hlists.

Yea, pretty sure at this point that the clash happens due to ambiguity of the inner fields that happen to be both identical and transmogrifiable (i.e. identical labelled HLists or a fields with the same name and same type that is also LabelledGeneric); the two things are not mutually exclusive :(

Not entirely sure how to get around it at this point. Any ideas?

ExpHP · 2018-09-24T14:12:40Z

I'm not entirely sure I understand the purpose of PluckedValue, but this test has me worried:

fn test_transmogrify_simple_identity() {
    let one: PluckedValue<i32> = PluckedValue(1);
    let one_again: i32 = one.transmogrify();
    assert_eq!(one_again, 1);
}

I can't put my finger on exactly why I feel this way... but it feels to me like there are demons lurking just beneath the surface here, waiting for their chance to jump out and cause type inference errors.

lloydmeta · 2018-09-24T14:58:50Z

@ExpHP Thanks for the quick review :) IIRC I had to do use it in order to get make it so that the following 3 wouldn't clash with the non-trivial case where we needed to pluck things out.

/// Implementation of Transmogrifier for when the target is empty and the Source is non-empty
impl<SourceHead, SourceTail> Transmogrifier<HNil, HNil> for HCons<SourceHead, SourceTail> {
    #[inline(always)]
    fn transmogrify(self) -> HNil {
        HNil
    }
}

As I mentioned in the reply to your comment, the straight labelled-HList version doesn't compile at this point (though I think at sometime during my experimentation, I had something like that working), but the following, with LabelledGeneric s does, even though it's largely the same structurally.

Identical structure transform

#[derive(LabelledGeneric)]
struct InternalPerson<'a> {
    name: &'a str,
    age: usize,
    is_banned: bool,
}

#[derive(LabelledGeneric, Debug)]
struct ExternalPerson<'a> {
    name: &'a str,
    age: usize,
    is_banned: bool,
}

let internal_user = InternalPerson {
    name: "John",
    age: 10,
    is_banned: true,
};

let external_user: ExternalPerson = internal_user.transmogrify();
println!("{:?}", external_user);

Non-identity transform where a labelled field gets an identity transform

#[derive(LabelledGeneric)]
struct InternalPhoneNumber {
    emergency: Option<usize>,
    main: usize,
    secondary: Option<usize>,
}

#[derive(LabelledGeneric)]
struct InternalAddress<'a> {
    is_whitelisted: bool,
    name: &'a str,
    phone: InternalPhoneNumber,
}

#[derive(LabelledGeneric)]
struct InternalPerson<'a> {
    name: &'a str,
    age: usize,
    address: InternalAddress<'a>,
    is_banned: bool,
}

#[derive(LabelledGeneric, Debug)]
struct ExternalPhoneNumber {
    main: usize,
}

#[derive(LabelledGeneric, Debug)]
struct ExternalAddress<'a> {
    name: &'a str,
    phone: ExternalPhoneNumber,
}

#[derive(LabelledGeneric, Debug)]
struct ExternalPerson<'a> {
    name: &'a str,
    age: usize,
    address: ExternalAddress<'a>,
    is_banned: bool,
}


let internal_user = InternalPerson {
    name: "John",
    age: 10,
    address: InternalAddress {
        is_whitelisted: true,
        name: "somewhere out there",
        phone: InternalPhoneNumber {
            main: 1234,
            secondary: None,
            emergency: Some(1234),
        },
    },
    is_banned: true,
};

let external_user: ExternalPerson = internal_user.transmogrify();
println!("{:?}", external_user);

It would be really awesome if you could check out this branch and kick the tires so to speak and play around with it locally to see if you could figure out where I went wrong here and see if we can't get the straight-labelled HList version working (again) too; getting it working to this point for the LabelledGeneric usage my main aim and it was already quite taxing on my type-foo-mana 😅 .

lloydmeta · 2018-09-24T15:05:23Z

Adding a PoC label as well just to make it clear this is early stages and needs all the help it can get :)

Centril

I haven't looked over the wider semantic type inference implications here and I'll leave that to ExpHP; I have some nits and some unrelated thoughts however which came to me while reading this. I would generally go over the punctuation and backticks in the PR.

Centril · 2018-09-25T01:50:40Z

core/src/coproduct.rs

-    Rem,
-> CoproductSubsetter<Coproduct<THead, TTail>, HCons<NHead, NTail>> for Choices
+impl<Choices, THead, TTail, NHead, NTail, Rem>
+    CoproductSubsetter<Coproduct<THead, TTail>, HCons<NHead, NTail>> for Choices


Nice rustfmt improvements!

Centril · 2018-09-25T01:54:07Z

core/src/hlist.rs

+    #[deprecated(
+        since = "0.1.30",
+        note = "Please use len() or static_len() instead."
+    )]


Unrelated to this PR... Should we make this go poof in the next breaking release at some point? If so let's create a tracking issue for it. :)

Yes we most definitely should ! #125

Centril · 2018-09-25T01:59:55Z

core/src/labelled.rs

@@ -65,10 +64,95 @@
 //! let s_user: ShortUser = frunk::transform_from(n_user); // done
 //! # }
 //! ```
+//!
+//! If you have the need to transform types that are similarly-shaped recursively, then
+//! use the Transmogrifier trait


End of sentence punctuation missing.

core/src/labelled.rs

Centril · 2018-09-25T02:02:22Z

core/src/labelled.rs

+//! /// ExternalUser, taking care of subfield conversions as well.
+//! let external_user: ExternalUser = internal_user.transmogrify();
+//!
+//! println!("{:?}", external_user);


I'd remove this line as it is redundant for explanation / testing purposes.

Centril · 2018-09-25T02:12:43Z

core/src/labelled.rs

+    }
+}
+
+/// Implementation of Transmogrifier for when the target is empty and the Source is non-empty


Punctuation + backticks.

Centril · 2018-09-25T02:12:54Z

core/src/labelled.rs

+    }
+}
+
+/// Implementation of Transmogrifier for when the target is an HList, and the Source is a Plucked HList


Punctuation + backticks.

Centril · 2018-09-25T02:13:20Z

core/src/labelled.rs

+    }
+}
+
+/// For the case where we need to do work in order to transmogrify one type into another


Punctuation.

Centril · 2018-09-25T02:13:42Z

core/src/labelled.rs

+        TargetTail,
+        PluckSourceHeadNameIndex,
+        TransMogSourceHeadValueIndices,
+        TransMogTailIndices,


Oh dear lord =P

Centril · 2018-09-25T02:17:05Z

core/src/labelled.rs

+    fn test_transmogrify_hcons_sculpting_super_simple() {
+        type Source = Hlist![Field<name, &'static str>, Field<age, i32>, Field<is_admin, bool>];
+        type Target = Hlist![Field<age, i32>];
+        let hcons: Source = hlist!(field!(name, "joe"), field!(age, 3), field!(is_admin, true));


I wonder if we should add a macro:

record! { // bikeshed name: "joe", age: 3, is_admin: true, }

Possibly also:

Record! { // bikeshed name: &'static str, age: i32, is_admin: bool, }

Good idea ! Filed #126

lloydmeta · 2018-09-25T06:48:53Z

@ExpHP so, I managed to get rid of the PluckedValue thing by leaning on Field<Name, Value> as a wrapper instead :) Thanks for pushing me towards getting rid of one more unnecessary task :)

@Centril

@Centril Thanks for the thorough review ! Went through and touched up the docs with punctuations and back ticks as you suggested :)

extracted from `InternalUser` do not implement `LabelledGeneric`, not does the values exptression extracted from `ExternalUser`: For some reason, the compiler does not seem to know to go one chunk at a time, solving for `&str: Transmogrify`, without having the whole thing transmogrify ``` error[E0277]: the trait bound `frunk::HCons<&str, frunk::HCons<ExternalCredentials, frunk::HCons<&str, frunk::HNil>>>: frunk::LabelledGeneric` is not satisfied --> examples/labelled.rs:76:50 | 76 | let external_user: ExternalUser = inner_user.transmogrify(); | ^^^^^^^^^^^^ the trait `frunk::LabelledGeneric` is not implemented for `frunk::HCons<&str, frunk::HCons<ExternalCredentials, frunk::HCons<&str, frunk::HNil>>>` | = note: required because of the requirements on the impl of `frunk::labelled::Transmogrifier<frunk::HCons<&str, frunk::HCons<ExternalCredentials, frunk::HCons<&str, frunk::HNil>>>, frunk::indices::There<_>, frunk::indices::There<frunk::HCons<frunk::indices::There<frunk::indices::Here>, frunk::HCons<frunk::indices::There<frunk::indices::Here>, frunk::HCons<frunk::indices::Here, frunk::HNil>>>>>` for `frunk::HCons<&str, frunk::HCons<InternalCredentials, frunk::HCons<&str, frunk::HNil>>>` = note: required because of the requirements on the impl of `frunk::labelled::Transmogrifier<ExternalUser<'_>, frunk::indices::There<frunk::indices::There<_>>, frunk::indices::There<frunk::HCons<frunk::indices::There<frunk::indices::Here>, frunk::HCons<frunk::indices::There<frunk::indices::Here>, frunk::HCons<frunk::indices::Here, frunk::HNil>>>>>` for `InternalUser<'_>` error[E0277]: the trait bound `frunk::HCons<&str, frunk::HCons<InternalCredentials, frunk::HCons<&str, frunk::HNil>>>: frunk::LabelledGeneric` is not satisfied --> examples/labelled.rs:76:50 | 76 | let external_user: ExternalUser = inner_user.transmogrify(); | ^^^^^^^^^^^^ the trait `frunk::LabelledGeneric` is not implemented for `frunk::HCons<&str, frunk::HCons<InternalCredentials, frunk::HCons<&str, frunk::HNil>>>` | = note: required because of the requirements on the impl of `frunk::labelled::Transmogrifier<frunk::HCons<&str, frunk::HCons<ExternalCredentials, frunk::HCons<&str, frunk::HNil>>>, frunk::indices::There<_>, frunk::indices::There<frunk::HCons<frunk::indices::There<frunk::indices::Here>, frunk::HCons<frunk::indices::There<frunk::indices::Here>, frunk::HCons<frunk::indices::Here, frunk::HNil>>>>>` for `frunk::HCons<&str, frunk::HCons<InternalCredentials, frunk::HCons<&str, frunk::HNil>>>` = note: required because of the requirements on the impl of `frunk::labelled::Transmogrifier<ExternalUser<'_>, frunk::indices::There<frunk::indices::There<_>>, frunk::indices::There<frunk::HCons<frunk::indices::There<frunk::indices::Here>, frunk::HCons<frunk::indices::There<frunk::indices::Here>, frunk::HCons<frunk::indices::Here, frunk::HNil>>>>>` for `InternalUser<'_>` ```

… HLists

Managed to get rid of PluckedValue; the other tests proposed still don't work as explained, but I think we can improve on it later better than not having this at all? Open for another review :)

ExpHP · 2018-10-03T23:14:21Z

So, unfortunately, it doesn't really surprise me that one of those tests didn't work, and I'm pretty sure it's impossible to fix. The fundamental problem is that the only feasible way to let primitive and std types like u32 transform into u32 is with a blanket impl for T -> T, and this must necessarily overlap with the specialized impls on HList.

Well, I shouldn't say that's the only feasible way. It's clear from the Haskell package's README how they handle it:

An explicit list of types is needed to be able to stop type recursion; this is currently limited to numeric types and Char.

which is a pretty serious limitation.

IMO, it is too easy to see the man behind the curtain, and it's too easy to break working impls by removing a field from the source type or adding one to the target type.

ExpHP · 2018-10-03T23:14:34Z

Absolutely insane idea. Make of this what you will.

#[derive(FlatLabelledGeneric)]
struct Field {
    apples: u32,
    oranges: u32,
}

#[derive(FlatLabelledGeneric)]
struct Thing {
    number: u32,
    #[frunk(flatten)]
    field: Field,
    string: String,
}

// <Thing as FlatLabelledGeneric>::Repr will be
//
// Hlist![
//     field!("number", u32),
//     field!("field.apples", u32),
//     field!("field.oranges", u32),
//     field!("string", String),
// ]
//
// where field!("string literal", ...) is understood to construct the type-level
// string corresponding to that literal
//
// the fields added through #[frunk(flatten)] are produced without
// knowledge of that type's definition, with the help of this trait:

/// Prefixes every field name in an HList of Fields with a fixed string.
trait PrefixFieldLabels<PrefixStr> {
    type Prefixed;

    fn prefix_field_labels(self) -> Self::Prefixed;
}

With that, the implementation of transmogrify becomes:

let repr = FlatLabelledGeneric::into(self);
let mogged = repr.sculpt();
FlatLabelledGeneric::from(mogged)

and correctly handles all fields, at the cost of the user needing to maintain the #[frunk(flatten)] tags.

ExpHP · 2018-10-03T19:57:59Z

core/src/labelled.rs

+    fn transmogrify(self) -> Target;
+}
+
+/// For the case where we don't need to do any transmogrifying at all because the source


I decided to look at the new doc page for frunk_core::labelled and it's pretty intimidating due to things like this. I wonder if this and other related index types should be moved to indices?

The headline should also mention that it is an index type.

Good call. Moved :)

* Add examples to the readme * Add examples to the main lib doc

lloydmeta · 2018-10-04T12:39:50Z

Absolutely insane idea. Make of this what you will.

That's a really cool idea :D I'd probably need a bunch more time to explore it and build something that works 😄 I think one challenge there might be finding a way to unflatten the keys back out; super interesting though and I'm keen to explore in a follow up attempt to improve this !

lloydmeta · 2018-10-09T16:47:50Z

Gonna merge this in for now since it feels mostly Good Enough and I don't want Perfect to be the enemy of Good. ; I think the overall traits + type-param signatures will probably remain more-or-less the same and we can always re-iterate on the implementation later.

lloydmeta added enhancement discussion labels Sep 24, 2018

lloydmeta self-assigned this Sep 24, 2018

lloydmeta requested review from Centril and ExpHP September 24, 2018 13:08

ExpHP previously requested changes Sep 24, 2018

View reviewed changes

lloydmeta added PoC help wanted labels Sep 24, 2018

Centril previously requested changes Sep 25, 2018

View reviewed changes

lloydmeta force-pushed the feature/transmogrify-redux2 branch from 4e5a8bb to 560c191 Compare September 25, 2018 13:23

lloydmeta added 15 commits October 2, 2018 21:45

Add Keys trait and impl for labelled hlists

86c1643

* Add ByKeyPlucker

f92ccf5

* Add ByKeySculptor

8f127fc

* Add a comment and attribute

8af677b

* Add ZipWithKeys

9136cf5

stash

523ece1

borked

4670626

Transmorgify stash

75fe067

Made some progress; need to define Keys and IntoUnlabelled for nested…

47b76c4

… HLists

Moar

4735f44

* Compiles

00dd00a

Manual annotation works...

e7b301f

Interesting; it almost works, but only when you don't need all fields

cd3f578

more tweaking

6ad795f

lloydmeta added 14 commits October 2, 2018 21:45

Add doc

a88e705

bbq indeed.

95f4d03

Great Scott

70c070c

* Removed unuseds traits and impls

7cb42b6

* Move a few things around, improve docs

7b860a5

* Add a doctest on the trait

4cd15f8

* More docs

51e4f39

* Remove PluckedValue wrapper

b352dda

* Various punctuation fixes

59a8b45

One more punctuation fix.

60d5c07

* Remove redundant println

5d09c4b

* Improve example

1b2b48f

* Add a top-level test

93aa11b

Pretty debug print

c6b8b1f

lloydmeta force-pushed the feature/transmogrify-redux2 branch from 4d18860 to c6b8b1f Compare October 2, 2018 12:45

Add benches

1b8dedf

lloydmeta requested a review from ExpHP October 3, 2018 14:50

ExpHP reviewed Oct 3, 2018

View reviewed changes

* Move newly added indices to the indices module

d7b8178

* Add examples to the readme * Add examples to the main lib doc

Centril mentioned this pull request Oct 4, 2018

add Selector::get_mut #130

Merged

Merge branch 'master' into feature/transmogrify-redux2

5a32bae

lloydmeta merged commit 960416d into master Oct 9, 2018

lloydmeta deleted the feature/transmogrify-redux2 branch October 9, 2018 16:48

lloydmeta mentioned this pull request Nov 21, 2018

[DEBUG] Try fixing nightly compile #136

Merged

ExpHP mentioned this pull request Jan 22, 2019

Transmogrification fails if common field implements LabelledGeneric #143

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce Transmogrifying from A to B #124

Introduce Transmogrifying from A to B #124

lloydmeta commented Sep 24, 2018

ExpHP Sep 24, 2018

lloydmeta Sep 24, 2018

lloydmeta Sep 25, 2018 •

edited

Loading

ExpHP commented Sep 24, 2018 •

edited

Loading

lloydmeta commented Sep 24, 2018

lloydmeta commented Sep 24, 2018

Centril left a comment

Centril Sep 25, 2018

Centril Sep 25, 2018

lloydmeta Sep 25, 2018 •

edited

Loading

Centril Sep 25, 2018

Centril Sep 25, 2018

Centril Sep 25, 2018

Centril Sep 25, 2018

Centril Sep 25, 2018

Centril Sep 25, 2018

Centril Sep 25, 2018

lloydmeta Sep 25, 2018

lloydmeta commented Sep 25, 2018

ExpHP commented Oct 3, 2018

ExpHP commented Oct 3, 2018 •

edited

Loading

ExpHP Oct 3, 2018

lloydmeta Oct 4, 2018

lloydmeta commented Oct 4, 2018

lloydmeta commented Oct 9, 2018

Introduce Transmogrifying from A to B #124

Introduce Transmogrifying from A to B #124

Conversation

lloydmeta commented Sep 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lloydmeta Sep 25, 2018 • edited Loading

Choose a reason for hiding this comment

ExpHP commented Sep 24, 2018 • edited Loading

lloydmeta commented Sep 24, 2018

lloydmeta commented Sep 24, 2018

Centril left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lloydmeta Sep 25, 2018 • edited Loading

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

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

Choose a reason for hiding this comment

lloydmeta commented Sep 25, 2018

ExpHP commented Oct 3, 2018

ExpHP commented Oct 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lloydmeta commented Oct 4, 2018

lloydmeta commented Oct 9, 2018

lloydmeta Sep 25, 2018 •

edited

Loading

ExpHP commented Sep 24, 2018 •

edited

Loading

lloydmeta Sep 25, 2018 •

edited

Loading

ExpHP commented Oct 3, 2018 •

edited

Loading