Revisit the representation of variant id for adjacently tagged enums

[dtolnay]

Currently the Serialize impl for an untagged enum such as:

#[derive(Serialize, Deserialize)]
#[serde(tag = "t", content = "c")]
enum Enum {
    Variant(T),
    /* ... */
}

looks like this:

impl Serialize for Enum {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: Serializer,
    {
        match self {
            Enum::Variant(__field0) => {
                let mut __struct = serializer.serialize_struct("Enum", 2)?;
                __struct.serialize_field("t", "Variant")?;
                __struct.serialize_field("c", __field0)?;
                __struct.end()
            }
            /* ... */
        }
    }
}

i.e. the variant id is simply serialized using impl Serialize for str.

During deserialization, that "Variant" is deserialized by this type:

enum __VariantIdentifier {
    Variant,
    /* ... */
}

struct __VariantIdentifierVisitor;

const __VARIANTS: &'static [&'static str] = &["Variant", /* ... */];

impl<'de> Visitor<'de> for __VariantIdentifierVisitor {
    type Value = __VariantIdentifier;

    fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
        formatter.write_str("variant identifier")
    }

    fn visit_u64<E>(self, value: u64) -> Result<Self::Value, E>
    where
        E: serde::de::Error,
    {
        match value {
            0u64 => Ok(__VariantIdentifier::Variant),
            /* ... */
            _ => Err(E::invalid_value(
                serde::de::Unexpected::Unsigned(value),
                &"variant index 0 <= i < 1",
            )),
        }
    }

    fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
    where
        E: serde::de::Error,
    {
        match value {
            "Variant" => Ok(__VariantIdentifier::Variant),
            /* ... */
            _ => Err(E::unknown_variant(value, __VARIANTS)),
        }
    }

    fn visit_bytes<E>(self, value: &[u8]) -> Result<Self::Value, E>
    where
        E: serde::de::Error,
    {
        match value {
            b"Variant" => Ok(__VariantIdentifier::Variant),
            /* ... */
            _ => Err(E::unknown_variant(value, __VARIANTS)),
        }
    }
}

impl<'de> Deserialize<'de> for __VariantIdentifier {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: Deserializer<'de>,
    {
        deserializer.deserialize_identifier(__VariantIdentifierVisitor)
    }
}

Notice at least 2 things about this don't make sense:

1. The presence of visit_u64. There is no possible way that a data format could have turned "Variant" into 0. This is neither a struct field (which are sometimes numbered in order by data formats) nor an externally tagged serde enum (for which serde passes a numeric id into serialize_*_variant, in addition to the string variant name).

2. The use of deserialize_identifier. In formats that serialize externally tagged enums using the variant number instead of name, deserialize_identifier involves reading a number. It doesn't make sense to serialize something as a str and deserialize as number. Possibly the best thing that could happen, depending on the specific binary format, is that it takes the length of the variant name as if that were the variant index.

We should make one of the following changes:

Deserialize using deserialize_str, delete visit_u64

Keep serializing using serialize_str, and deserialize using deserialize_str instead of deserialize_identifier.

This would fix the broken behavior in formats where deserialize_identifier is not the same as deserialize_str.

Serialize using an externally tagged unit variant

In this approach, the following adjacently tagged enum:

#[derive(Serialize)]
#[serde(tag = "t", content = "c")]
enum Enum {
    Variant(T),
    /* ... */
}

would be serialized as if it were effectively represented by:

#[derive(Serialize)]
struct Enum {
    t: EnumTag,
    c: EnumContent,
}

#[derive(Serialize)]
enum EnumTag {
    Variant,
    /* ... */
}

#[derive(Serialize)]
#[serde(untagged)]
enum EnumContent {
    Variant(T),
    /* ... */
}

This way non-human-readable formats automatically serialize just an integer id for the variant identifier for the adjacently tagged enum, same as they do for externally tagged enums. This should be faster and more compact than serializing the whole variant names as strings.

[Mingun]

Your point 1 is what exactly was required in #2479 so you should think twice about removing this ability

[dtolnay]

I don't immediately see the connection. That issue appears to be about deserializing the "t" vs "c" from a number. This issue is about deserializing the "Variant" from a number.

[Mingun]

Yes, you're right, but you does not right that this is always impossible. Yes, it is impossible right now, but if ability to use arbitrary compile-time expressions in tag and content fields will be permitted (which seems reasonable if we want to permit them in rename attributes), then the one could write:

#[derive(Serialize, Deserialize)]
#[serde(tag = 1, content = 2)]
enum AdjacentlyTagged {
  // ...
}

The related issues for rename are #745, #1773, #1964, #2358, #2485

[Baptistemontan]

#2056 seems also related