Use #[non_exhaustive] instead of manual variant

[niclashoyer]

first issue 🎉

Fixes #397

[Dirbaio]

Very nice!

Can you update .github/workflows/test.yml and README.md?

[niclashoyer]

I'll try to, but I think there are other issues here. I just noticed that locally the tests run fine (I'm not sure if other feature combinations might fail, I can check that). But cargo clippy fails because I removed all _ => ... match arms.

I think clippy demands these wildcard arms, but #![cfg_attr(deny(unused))] demands removal of these match arms, since the code is unreachable. I'm not sure how to proceed here.

[Dirbaio]

You can make the match arms themselves conditional. For example instead of _ => None, you can do:

#[cfg(feature = "proto-ipv6")]
IpCidr::Ipv6 => None

[niclashoyer]

ok, I'll check that. README and workflow is updated now 🤞

[crawford]

Are we okay bumping the minimum Rust version? I believe @whitequark and https://github.com/m-labs/ have a fork of Rust that they use with this project.

[niclashoyer]

@crawford this was also mentioned in the issue: #397 (comment)

Meanwhile I need help again 😅

Adding the cfg attr to other match arms helped, but now I'm stuck at this macro in src/socket/mod.rs:

macro_rules! from_socket {
    ($socket:ty, $variant:ident) => {
        impl<'a, 'b> AnySocket<'a, 'b> for $socket {
            fn downcast<'c>(ref_: SocketRef<'c, Socket<'a, 'b>>) ->
                           Option<SocketRef<'c, Self>> {
                match SocketRef::into_inner(ref_) {
                    &mut Socket::$variant(ref mut socket) => Some(SocketRef::new(socket)),
                    _ => None,
                }
            }
        }
    }
}

This wildcard matches all other arms that are not given by the macro. Adding all other arms here would complicate this a lot, wouldn't it?

[Dirbaio]

@whitequark ACKed the MSRV bump #397 (comment) .

I'm also for bumping the MSRV. 1.36 is ancient and 1.40 is already a year old.

Additionally, this change is not only cosmetic. The extra dummy variants impact code size and struct layout.

[Dirbaio]

Adding the cfg attr to other match arms helped, but now I'm stuck at this macro in src/socket/mod.rs:

I think using if let should work there?

if let Socket::$variant(ref mut socket) = SocketRef::into_inner(ref_) { 
    Some(...)
} else {
    None
}

[Dirbaio]

Maybe that still triggers some warnings if the else branch is unreachable, in this case I'd be OK with some #[allow(..)] on that if...

[niclashoyer]

Better now. I'm still a bit lost here:

https://github.com/smoltcp-rs/smoltcp/pull/409/checks?check_run_id=1671886625

It seems that TcpState is behind socket-tcp which seems to trigger unused lifetimes. Is this new in 1.40?

Edit: no, master compiles fine on 1.40 using cargo test --no-default-features --features "std ethernet proto-ipv6 socket-tcp".

[Dirbaio]

Oh, that's very unfortunate.

With conditional compilation 'b is unused when just socket-tcp is enabled... Not sure what's the best solution.

[niclashoyer]

ah this issue seemed like a low hanging fruit 🍒 I'll go to bed now, maybe I've got more ideas tomorrow...

[Dirbaio]

Actually it turns out that having 2 lifetimes in sockets isn't really needed.

I've opened PR #410 to join them in just 'a. With that, the problem goes away :)

[Dirbaio]

Thank you for the PR @niclashoyer !