-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enums derive #71
Comments
Hi @elpiel yea this seems like a pretty cool idea! I will note I'm worried it's turning into a kind of serialization format, but your usecase seems reasonable; especially with the more C-ish enum types. Maybe it might help to give a list of enum use-cases you're interested in supporting (e.g., will this do full blown algebraic data type serialization/deserialization?) For simple
will look like. Looks like you've already got some interesting ideas for tagging, etc. Would you like to expand a bit more, maybe we can flesh out something that would make a PR more easier to evaluate and review whether it closes and/or "fixes" this issue :) And glad this library is working out for you in your project, can you share it? (Or is it the backlinked issue referencing it above ?) |
Hey @m4b 👋
The idea was just that 😅 - an idea. I like how Regarding:
What we need for our use case is, smth like enum MyEnum {
Case1 = 1,
Case2, // 2
Case3, // 3
Case56 = 56,
Case57, // 57 I haven't thought about that much, so it would be nice to hear your thoughts on this as well. |
So looking at your Feature enum, it carries data. To summarize, hopefully correctly, it looks like effectively you check for tag, then if it matches known lookup-table of tags, then goto branch for that tag, recursively. E.g., in the case of It might be useful to study this wire format: https://github.com/mfp/extprot/blob/master/doc/encoding.md#sum-types-disjoint-unions Iirc it is similar style for OCaml serialization of tagged unions and the data they can carry. I recall encountering a kind of deserialization ambiguity in some cases for tagged union wire formats, but I can't remember if it was just regular issues, or something else that doesn't matter that much. Anyway, might be good to consider above link, then perhaps for extended version of your example: #[derive(Pread, Pwrite)]
#[scroll(tag = "u16")]
enum MyEnum {
#[scroll(tag_value = "0")]
Case(CaseStruct),
#[scroll(tag_value = "1")]
CaseTwo(CaseTwoStruct),
#[scroll(tag_value = "2")]
CaseThree,
} we can start discussion about implementation details, e.g. what does the wire/binary format look like here? What error messages do we expect if we omit the `#[scroll(tag_value= "0")`` annotation (will it compile?). Can we omit the tag values and have a sensible default (this could/would be nice?)? Etc. But I think we could come up with something sensible perhaps, so you could theoretically slap a derive on an enum with complicated structure, and have it do something more or less meaningful; at the very least i think we could expect that a regular: #[repr(usize)]
#[derive(Pread, Pwrite)]
enum CThing {
Foo = 0,
Bar = 1,
Baz = 2,
} would do more or less precisely what you expect it to. (Probably we have to enforce that all numeric values are given, but yea, one gets the idea) |
I will take a look and read the wire format link you've provided! And yes, you are correct on your summary.
Regarding this part. It might be sensible to stick to how Rust handles this. Even if you don't specify the value For me this is a reasonable default and in this case you'll only need to specify that your Enum should be tagged. I think this is nice! In our case, the Thank you for your fast response and help! <3 |
Yes so the issue here (unless I'm missing something) is that when you go to deserialize in the derive implementation for i.e. #[repr(u16)]
enum Specified {
Foo = 0,
Bar = 6,
Baz,
Fizz,
} if we encounter 17, is it |
@m4b Discriminant values are stable for fieldless enums: https://doc.rust-lang.org/reference/items/enumerations.html#custom-discriminant-values-for-fieldless-enumerations |
I didn't see anything about it being stable, though I suppose the reference makes it so? And from link:
How do we know when the compiler used a smaller type, and did not? Probably the derive would have to require |
In any event, we could just by fiat make the un-explicitly tagged variants behave similar/identical to the reference posted (auto-incrementing +1 from last variant), for the derive serialization format of simple enums; this is probably what @elpiel was suggesting. |
Also this brings up question of what size the tag should be serialized as; e.g., in example above, we have: #[repr(u16)]
enum Foo {
Bar,
Baz
} I assume we'd serialize the tag here as a |
Disclaimer: I don't fully understand We don't actually need this right? How Rust is representing the enum can be different than what we (de)serialize from/into. Plus this can't apply for enums containing fields only for Fieldless enums. Maybe I don't fully understand it, so feel free to correct me 😅 |
Hello everyone!
I am working on a project in which we have enums that whey (de)serialized add the
Enum::Case(CaseStruct)
as au8
oru16
before (de)serializing theCaseStruct
.Currently we've implemented custom
TryFromCtx
&TryIntoCtx
, but it would be nice to be able to derive it and add something likeserde
enumtag
attribute and specify the type per case(maybe?!).The text was updated successfully, but these errors were encountered: