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

Fix unused_results complaining rustc lint in codegen for adjacently tagged enum #2116

Merged
merged 1 commit into from Dec 9, 2021
Merged

Fix unused_results complaining rustc lint in codegen for adjacently tagged enum #2116

merged 1 commit into from Dec 9, 2021

Conversation

tyranron
Copy link
Contributor

MRE

#![allow(dead_code)]
#![deny(unused_results)]

use serde::Deserialize; // =1.0.130

#[derive(Deserialize)]
#[serde(tag = "msg", content = "data")]
enum ServerMsg {
    Ping(u32),
    Event {
        room_id: u32,
    },
}

Playground

@tyranron
Copy link
Contributor Author

tyranron commented Oct 29, 2021

I'd like also to add that MRE as a test case, but couldn't figure out where is the right place for it in tests suite.

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Could you help me understand this PR? The code that is being emitted inside this match arm is:

=> {
    match serde::de::MapAccess::next_value::<serde::de::IgnoredAny>(&mut __map) {
        serde::__private::Ok(__val) => __val,
        serde::__private::Err(__err) => {
            return serde::__private::Err(__err);
        }
    };
    continue;
}

The signature of next_value is: https://docs.rs/serde/1.0.130/serde/de/trait.MapAccess.html#method.next_value. So next_value::<IgnoredAny>() is of type Result<IgnoredAny, E>, and __val (and therefore that whole match expression) is of type IgnoredAny. Where is there an unused result? Isn't this a rustc bug, not a serde_derive bug?

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Ah it's not saying there is an unused Result — that would be unused_must_use, not unused_result. 😄 It's saying there is a value of some expression, in this case of type IgnoredAny, which is unused.

Thanks, this looks good!

@dtolnay dtolnay merged commit 549fac7 into serde-rs:master Dec 9, 2021
@dtolnay
Copy link
Member

dtolnay commented Dec 9, 2021

I pushed fb2fe40 with an alternative fix because this one solves the rustc lint but causes a clippy lint.

error: calls to `std::mem::drop` with a value that implements `Copy`. Dropping a copy leaves the original intact
   --> test_suite/tests/test_gen.rs:468:25
    |
468 |     #[derive(Serialize, Deserialize)]
    |                         ^^^^^^^^^^^
    |
    = note: `-D clippy::drop-copy` implied by `-D clippy::all`
note: argument has type _::_serde::de::IgnoredAny
   --> test_suite/tests/test_gen.rs:468:25
    |
468 |     #[derive(Serialize, Deserialize)]
    |                         ^^^^^^^^^^^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#drop_copy
    = note: this error originates in the derive macro `Deserialize` (in Nightly builds, run with -Z macro-backtrace for more info)

@tyranron tyranron deleted the fix-unused-results branch December 9, 2021 13:43
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.

None yet

2 participants