Skip to content
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

Add exhaustive match arm in #[derive(Serialize)] expansion #2361

Closed

Conversation

mrlegohead0x45
Copy link

Closes #2360

.iter()
.enumerate()
.map(|(variant_index, variant)| {
serialize_variant(params, variant, variant_index as u32, cattrs)
})
.collect();

// unreachable because all the variants are matched exhaustively above
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is matching here on the remote enum, i.e., the one which does not implement Serialize. In your original issue that is the io::ErrorKind enum which is marked as non_exhaustive. So this reasoning does not work here. You never know for sure that you covered all enum variants, since any dependency update could change it.

It might be possible to add a catch-all variant to the def enum. All enum variants which match will be translated 1-to-1. The catchall arm _ then goes to a separate enum variant.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies if I still misunderstand you, but looking at the expansion of #[derive(Serialize)] on _StdIoErrorKindDef using vscode rust-analyzer expand macro recursively it looks like this

impl _StdIoErrorKindDef {
    fn serialize<__S>(
        __self: &io::ErrorKind,
        __serializer: __S,
    ) -> _serde::__private::Result<__S::Ok, __S::Error>
    where
        __S: _serde::Serializer,
    {
        // don't know what this bit does
        match _serde::__private::None::<&_StdIoErrorKindDef> {
            _ => {}
        }
        match _serde::__private::None {
            _serde::__private::Some(()) => {
                let _ = _StdIoErrorKindDef::NotFound;
            }
            _ => {}
        }
        // repeats for other variants

        // then
        match *__self {
            io::ErrorKind::NotFound => _serde::Serializer::serialize_unit_variant(
                __serializer,
                "_StdIoErrorKindDef",
                0u32,
                "NotFound",
            ),
            io::ErrorKind::PermissionDenied => _serde::Serializer::serialize_unit_variant(
                __serializer,
                "_StdIoErrorKindDef",
                1u32,
                "PermissionDenied",
            ),
            // for other variants
        }
    }
}

So it looks to me like it is matching against std::io::ErrorKind

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I misread your comment, so disregard my comment. I think though it would be fine as long as one kept the Def enum up to date with std::io:ErrorKind

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make it like this

#[derive(Serialize, Deserialize)]
#[serde(remote = "io::ErrorKind", wildcard_variant = "__Unrecognised")]
enum _StdIoErrorKindDef {
    // ...
    __Unrecognised
}

and then do _ => _StdIoErrorKindDef::__Unrecognised, and

#[derive(Serialize, Deserialize)]
#[serde(remote = "io::ErrorKind", wildcard = "unreachable")]
enum _StdIoErrorKindDef {
    // ...
}

for _ => unreachable!(), although I don't know how the first one would affect deserialisation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add wildcard arm when deriving Serialize for non exhaustive enums
2 participants