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

Mark generated impl de::Visitor blocks as #[automatically_derived] #2866

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

tdittr
Copy link
Contributor

@tdittr tdittr commented Dec 9, 2024

When using cargo llvm-cov on one of my projects I noticed that the #[derive(Deserialize)] annotations on my structs were marked as not covered.

This PR adds #[automatically_derived] also to the impl de::Visitor blocks that are generated. In my tests, they now no longer show up in the coverage as not covered.

rustc automatically excludes impl blocks marked with #[automatically_derived] since rust-lang/rust#120185.
The generated impl Deserialize block is already marked with #[automatically_derived] and thus correctly not counted.
However, the visitors and field visitors which are generated within the deserialize method were counted towards the coverage.

This hides the generated visitors and field visitors from code
coverage.
Copy link
Member

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

Thanks! That makes sense

@oli-obk oli-obk merged commit 46e9ecf into serde-rs:master Dec 9, 2024
15 checks passed
@tdittr tdittr deleted the mark-visitors-as-generated branch December 9, 2024 13:10
@dtolnay
Copy link
Member

dtolnay commented Dec 11, 2024

The generated impl Deserialize block is already marked with #[automatically_derived] and thus correctly not counted.

There were some Deserialize and other impls (DeserializeSeed, Serialize, remote deserialize/serialize) that were not marked. I have opened #2868 to add #[automatically_derived] to them all. Please let me know if there were any of those impls intentionally omitted from this PR and should not get #[automatically_derived]. I am not familiar with using llvm-cov in my own projects.

@tdittr
Copy link
Contributor Author

tdittr commented Dec 11, 2024

Oh interesting, I guess I just missed those. Thanks for also adding the annotation there!

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.

3 participants