Private reference in Value enum prevents exhaustive matching

[mplanchard]

The Other variant of the mlua::Value enumeration contains a private type, which means it is impossible to match on it like:

        mlua::Value::Nil => serde_json::Value::Null,
        mlua::Value::Boolean(b) => serde_json::Value::Bool(b),
        mlua::Value::Integer(i) => serde_json::Value::Number(i.into()),
        mlua::Value::Number(n) => {...}
        mlua::Value::String(s) => {...}
        mlua::Value::Table(t) => {...}
        mlua::Value::Function(_)
        | mlua::Value::LightUserData(_)
        | mlua::Value::Thread(_)
        | mlua::Value::UserData(_)
        | mlua::Value::Other(_)
        | mlua::Value::Error(_) => Err(anyhow::anyhow!(
            "Error converting Lua type to unsupported JSON type"
        ))

which yields:

rustc: error: type `mlua::types::value_ref::ValueRef` is private
   --> lib/rs/lib-workflow/src/lua/mod.rs:310:30
    |
310 |         | mlua::Value::Other(_)
    |                              ^ private type

We can just match on _, but we generally prefer exhaustive matches wherever possible, so that when new variants are introduced, we're sure to check to see whether we need to handle them.

[Pspritechologist]

I realize I'm a little late here, but I thought I'd share that matching was still possible by simply replacing the interior of Other with .. which acts to elide the pattern.

mlua::Value::Nil => serde_json::Value::Null,
mlua::Value::Boolean(b) => serde_json::Value::Bool(b),
mlua::Value::Integer(i) => serde_json::Value::Number(i.into()),
mlua::Value::Number(n) => {...}
mlua::Value::String(s) => {...}
mlua::Value::Table(t) => {...}
mlua::Value::Function(_)
| mlua::Value::LightUserData(_)
| mlua::Value::Thread(_)
| mlua::Value::UserData(_)
| mlua::Value::Other(..)
| mlua::Value::Error(_) => Err(anyhow::anyhow!(
    "Error converting Lua type to unsupported JSON type"
))

Yields no errors.

[mplanchard]

Huh, TIL.

I doubted this, so a small replication:

mod foo {

    struct Bar;

    pub enum Foo {
        A(String),
        #[allow(private_interfaces)]
        B(Bar),
    }
}

fn foo_bar(v: foo::Foo) {
    match v {
        foo::Foo::A(_) => {}
        foo::Foo::B(_) => {}
    }
}

yields

rustc: error: type `jobs::purge_old_event_summaries::foo::Bar` is private
   --> platform/spec-proxy-sidekick/src/jobs/purge_old_event_summaries.rs:454:21
    |
454 |         foo::Foo::B(_) => {}
    |                     ^ private type

But as you say:

mod foo {

    struct Bar;

    pub enum Foo {
        A(String),
        #[allow(private_interfaces)]
        B(Bar),
    }
}

fn foo_bar(v: foo::Foo) {
    match v {
        foo::Foo::A(_) => {}
        foo::Foo::B(..) => {}
    }
}

does not.

I guess it makes sense, since _ is sort of assigning the value, while .. is not, but it is definitely surprising, as I'd expect them to behave the same way.

[Pspritechologist]

I agree it's a little odd at first glance, but it makes a ton of sense really. As you said _ is still matching a value, it's identical to using any identifier. Vitally though, you should consider the effect it has on breaking changes. If matching a private field with _ was allowed then removing that field would be a breaking change, something which should not be the case. Using .. to elide it works no matter how many fields there are or if they ever go away.