lloydmeta / frunk

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

Add support for labelling TupleStructs by index #144

Closed bossmc closed 5 years ago

bossmc commented 5 years ago

This is a little unusual, since it basically undoes the good that LabelledGenerics by allowing someone to transmute in a way that switches the meaning of two members of the tuple but:

  1. This allows mixing tuple structs (especially "new-type" structs) into LabelledGenerics and still transmogrify freely.
  2. Tuple structs are "named" by their indexes, there's no better naming (unlike named structs where the field labels are meaningful)
  3. LabelledGeneric is opt-in so if there's a struct that's too primitive for transmogrifying to make sense (e..g Pair<T>(T, T)) then don't opt-in.
bossmc commented 5 years ago

Markups applied

ExpHP commented 5 years ago

Okay; supporting this makes sense from the standpoint of generic interfaces (i.e. writing a function that uses the semantics of LabelledGeneric for structs, and still supports tuple structs).


Should we similarly impl LabeledGeneric for tuples? (I think so, because they impl Generic.)


There is a question here of whether tuple structs should use underscore-prefixed names or not.

However, the difference may be moot, because ISTM there are vanishingly few places that can make use of either of these advantages. (these differences sound like they would be useful mostly in codegen, but codegen cannot make use of type information)

bossmc commented 5 years ago

Unit test - Yes, sure, there are no tests in the derive crate, where's the best place to test the derive code?

Field names - I'll admit I didn't think too hard about this... I went for _x since they're valid identifiers, but this does allow transmogrifying between Foo(String) and Bar { _0: String } which is a little surprising. Since it's not possible to define Bar { 0: String } the confusion disappears, but this precludes conversion from named structs to tuple structs forever...

LabelledGeneric for Tuples? Seems reasonable, does that block this PR?

ExpHP commented 5 years ago

I think tests for derives are in frunk/tests as integration tests (you can't unit test a proc macro from within it's own crate)

On Tue, Jan 29, 2019, 2:59 PM Andy Caldwell <notifications@github.com wrote:

Unit test - Yes, sure, there are no tests in the derive crate, where's the best place to test the derive code?

Field names - I'll admit I didn't think too hard about this... I went for _x since they're valid identifiers, but this does allow transmogrifying between Foo(String) and Bar { _0: String } which is a little surprising. Since it's not possible to define Bar { 0: String } the confusion disappears, but this precludes conversion from named structs to tuple structs forever...

LabelledGeneric for Tuples? Seems reasonable, does that block this PR?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/lloydmeta/frunk/pull/144#issuecomment-458685351, or mute the thread https://github.com/notifications/unsubscribe-auth/ABWI0HYlJuRALjlrepeAvpdCqg39jgA3ks5vIKgYgaJpZM4aM_6f .

lloydmeta commented 5 years ago

Unit test - Yes, sure, there are no tests in the derive crate, where's the best place to test the derive code? .. I think tests for derives are in frunk/tests as integration tests (you can't unit test a proc macro from within it's own crate)

Sorry yeah, I meant integration tests under /tests and/or doc tests ( I think they work in there for some reason ?). https://docs.rs/frunk/0.2.2/frunk/

this does allow transmogrifying between Foo(String) and Bar { _0: String } which is a little surprising. Since it's not possible to define Bar { 0: String } the confusion disappears, but this precludes conversion from named structs to tuple structs forever...

Good point; that is a bit surprising and yet .. precluding conversion from named structs to tuple structs might not be desirable? One could make the argument that if someone is naming their field _$someNumber that they might be doing it on purpose to allow conversion?

ExpHP commented 5 years ago

Okay, it's hard to predict which is the better choice. Since nobody seems to have a strong opinion the current implementation is probably fine for now.


Re: tuple impl, I don't think we need to block on it. I do think it should be added before the next release though.

ExpHP commented 5 years ago

Oh: add a note to the release notes under "Unreleased"

bossmc commented 5 years ago

I've added a couple of UTs (one for the use case I care about, that of new-type wrappers being interchangeable if the inner type is the same), and added an entry to the changelog.

lloydmeta commented 5 years ago

Released as 0.2.4 thanks for the great work 😄