serde icon indicating copy to clipboard operation
serde copied to clipboard

UPDATED: Integer/boolean tags for internally/adjacently tagged enums

Open Astavie opened this issue 2 years ago • 19 comments

this is basically just #2056 but synced with the latest serde master closes #745

for implementation details see #2056

Example

#[derive(Serialize, Deserialize, Debug)]
#[serde(tag = "op")]
enum Something {
    #[serde(rename = 1)]
    A { a: i32 },
    #[serde(rename = 2)]
    B(u64),
    #[serde(rename = true)]
    C,
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(tag = "op", content = "d")]
enum Adjacent {
    #[serde(rename = 1)]
    A { a: i32 },
    #[serde(rename = 2)]
    B(u64),
    #[serde(rename = true)]
    C,
}

Astavie avatar Jul 23 '23 13:07 Astavie

@dtolnay

Is there still any blocking point for this PR after update and pass CI? Using integers as tags is the feature I am most looking forward to and find useful.

7sDream avatar Jul 26 '23 03:07 7sDream

The new adjacently tagged enum code really put a wrench into this. It effectively now serializes tags into a unit enum like:

enum EnumTag {
    A,
    B,
    // ...
}

This way some (de)serializers can use the variant index instead of the name for smaller sizes. However there is no (de)serialize method that takes a variant index and a generic data type. The closest is serialize_newtype_variant but that wraps the newtype.

A new (de)serialize method could be created for this, something like serialize_unit_newtype_variant. However that would be a breaking change (unless given a default impl I suppose).

A default impl would simply redirect to the newtype's serialize impl I think.

Astavie avatar Aug 02 '23 20:08 Astavie

@dtolnay is making a new base serialization method a good solution here? It could even expand enum tags to more than booleans and integers later on and add support for non-string tags for unit enums.

Astavie avatar Aug 02 '23 20:08 Astavie

It not necessarily an obstacle, but it does mean I need to do a slight rework of the (de)serialization of tags. Your implementation suggestion actually makes a lot of sense, I'll work on that one. Also yes, a mix is currently supported.

Astavie avatar Aug 04 '23 09:08 Astavie

Alright, that should be up-to-date again now

Astavie avatar Aug 04 '23 12:08 Astavie

I've now done most of the simple changes. The rest of the requested changes will be completed on a later date

Astavie avatar Sep 22 '23 13:09 Astavie

@Astavie, that's an amazing piece of work, well done! May we help you somehow?

xamgore avatar Dec 25 '23 15:12 xamgore

@xamgore There still needs to be support for different type of integer tags (see https://github.com/serde-rs/serde/pull/2525#discussion_r1285288903). This requires a bit of a refactor on top of the refactor i've already done. If you're able to help with that it's much appreciated as I've had less time to devote to this. For the rest, tidying up some error messages as discussed before.

Astavie avatar Dec 27 '23 18:12 Astavie

Support for marking specific integer types has been added. Specifically: i8, i16, i32, i64, u8, u16, u32, u64

Rules for unmarked integers:

  • If all variants are unmarked integers, the smallest fitting integer type is chosen
  • If there exist variants with one integer type and the rest unmarked, that type will be chosen
  • If there is a mix of integer types or there are string variants or boolean variants, it will default to i64

Astavie avatar Feb 08 '24 15:02 Astavie

Is there ongoing work for this PR? I can help push this one over the line, as these features are highly valuable to my company. I've pulled @Astavie 's branch, merged master, and ran the test suite thus far. Any guidance would be greatly appreciated @dtolnay

jenr24-architect avatar May 12 '24 18:05 jenr24-architect

@jenr24-architect iirc, the only work required now are moving the highlight spot with the errors mentioned by @dtolnay above, though they also mentioned not having reviewed the deserialization code yet

Astavie avatar May 13 '24 08:05 Astavie

Excuse me for bothering, can someone give an advice about how I can implement similar logic in my code (i have different enum’s variants are differentiated by integer tag)

r4v3n6101 avatar Jun 26 '24 12:06 r4v3n6101

Is it broken?

Cargo.toml

[package]
name = "serde_tagged_enum"
version = "0.1.0"
edition = "2021"

[dependencies]
serde = { git = "https://github.com/Astavie/serde", branch = "integer-tags-for-enums", features = ["derive"] }
serde_json = "1"

main.rs

use serde::{Deserialize, Serialize};

#[derive(Debug, Default, Serialize, Deserialize)]
pub struct AttachmentUploadEntry {
    pub id: u32,
    pub upload_filename: String,
    pub upload_url: String,
}

fn main() {
    let data: String = "{}".to_string();
    let mut json: AttachmentUploadEntry = serde_json::from_str(&data).unwrap();
}

Error:

error[E0277]: the trait bound `AttachmentUploadEntry: serde::de::Deserialize<'_>` is not satisfied
    --> src/main.rs:12:43
     |
12   |     let mut json: AttachmentUploadEntry = serde_json::from_str(&data).unwrap();
     |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `serde::de::Deserialize<'_>` is not implemented for `AttachmentUploadEntry`
     |
     = note: for local types consider adding `#[derive(serde::Deserialize)]` to your `AttachmentUploadEntry` type
     = note: for types from other crates check whether the crate offers a `serde` feature flag
     = help: the following other types implement trait `serde::de::Deserialize<'de>`:
               &'a Path
               &'a [u8]
               &'a str
               ()
               (T,)
               (T0, T1)
               (T0, T1, T2)
               (T0, T1, T2, T3)
             and 142 others
note: required by a bound in `serde_json::from_str`
    --> /home/somobu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/serde_json-1.0.120/src/de.rs:2676:8
     |
2674 | pub fn from_str<'a, T>(s: &'a str) -> Result<T>
     |        -------- required by a bound in this function
2675 | where
2676 |     T: de::Deserialize<'a>,
     |        ^^^^^^^^^^^^^^^^^^^ required by this bound in `from_str`

IriaSomobu avatar Jul 13 '24 19:07 IriaSomobu

@IriaSomobu tried this myself, it's because serde_json uses the regular version of serde and not my fork, leading to serde_json not thinking the struct has a Deserialize implementation. you can fix this using patch.crates-io in Cargo.toml instead:

[package]
name = "serde_tagged_enum"
version = "0.1.0"
edition = "2021"

[dependencies]
serde = { version = "1", features = ["derive"] }
serde_json = "1"

[patch.crates-io]
serde = { git = "https://github.com/Astavie/serde.git", branch = "integer-tags-for-enums" }

note, cargo might complain about not finding a version for serde but just unlocking the dependencies by removing cargo.lock fixed that for me

Astavie avatar Jul 15 '24 12:07 Astavie

I see. Sorry for bothering you :)

IriaSomobu avatar Jul 21 '24 15:07 IriaSomobu