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
Revisit the representation of variant id for adjacently tagged enums #2496
Comments
Your point 1 is what exactly was required in #2479 so you should think twice about removing this ability |
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. |
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 #[derive(Serialize, Deserialize)]
#[serde(tag = 1, content = 2)]
enum AdjacentlyTagged {
// ...
} The related issues for rename are #745, #1773, #1964, #2358, #2485 |
#2056 seems also related |
Also see serde-rs/serde#2496 Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
* Bump serde from 1.0.180 to 1.0.183 in /lang/rust Bumps [serde](https://github.com/serde-rs/serde) from 1.0.180 to 1.0.183. - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](serde-rs/serde@v1.0.180...v1.0.183) --- updated-dependencies: - dependency-name: serde dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * Update the Serde impl after serde-rs/serde#2505 Also see serde-rs/serde#2496 Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
* Bump serde from 1.0.180 to 1.0.183 in /lang/rust Bumps [serde](https://github.com/serde-rs/serde) from 1.0.180 to 1.0.183. - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](serde-rs/serde@v1.0.180...v1.0.183) --- updated-dependencies: - dependency-name: serde dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * Update the Serde impl after serde-rs/serde#2505 Also see serde-rs/serde#2496 Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org> (cherry picked from commit e26943f)
Recent Serde changes ([1], [2]) changed the internal representation of enums, breaking our test. [1]: serde-rs/serde#2496 [2]: serde-rs/serde#2505
Recent Serde changes ([1], [2]) changed the internal representation of enums, breaking our test. [1]: serde-rs/serde#2496 [2]: serde-rs/serde#2505
Currently the
Serialize
impl for an untagged enum such as:looks like this:
i.e. the variant id is simply serialized using
impl Serialize for str
.During deserialization, that "Variant" is deserialized by this type:
Notice at least 2 things about this don't make sense:
The presence of
visit_u64
. There is no possible way that a data format could have turned "Variant" into0
. 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 intoserialize_*_variant
, in addition to the string variant name).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 astr
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
, deletevisit_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:
would be serialized as if it were effectively represented by:
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.
The text was updated successfully, but these errors were encountered: