Skip to content

Downgrade transmute_undefined_repr to nursery #8418

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

Merged
merged 1 commit into from
Feb 12, 2022

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Feb 12, 2022

Reason: #8417. I am skeptical of this lint but maybe there is a narrower subset of types on which it is useful, so keeping it for now but moving to nursery for further development.


Please write a short comment explaining your change (or "none" for internal only changes)

changelog: Remove [transmute_undefined_repr] from default set of enabled lints

@rust-highfive
Copy link

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 12, 2022
@llogiq
Copy link
Contributor

llogiq commented Feb 12, 2022

Thank you! @bors r+

@bors
Copy link
Contributor

bors commented Feb 12, 2022

📌 Commit 7964406 has been approved by llogiq

@bors
Copy link
Contributor

bors commented Feb 12, 2022

⌛ Testing commit 7964406 with merge 5707491...

@bors
Copy link
Contributor

bors commented Feb 12, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 5707491 to master...

@bors bors merged commit 5707491 into rust-lang:master Feb 12, 2022
@dtolnay dtolnay deleted the transmuteundefinedrepr branch February 12, 2022 07:44
bors added a commit that referenced this pull request Mar 2, 2022
…shearth

Transmute_undefined_repr to nursery again

This PR reinstates #8418, which was reverted in #8425 (incorrectly I think).

I don't want to start a revert war over this but I feel very strongly that this lint is not in a state that would be a net benefit to users of clippy. In its current form, making this an enabled-by-default `correctness` lint with authoritative-sounding proclamations of undefined behavior does more harm than the benefit of the true positive cases.

I can file a bunch more examples of false positives but I don't want to give the author of this lint the impression that it is ready to graduate from `nursery` as soon as I've exhausted the amount of time I am willing to spend revising this lint.

Instead I would recommend that the author of the lint try running it on some reputable codebases containing transmutes. Everywhere that the lint triggers please consider critically whether it should be triggering. For cases that you think are true positives, please raise a few of them with the crate authors (in a PR or issue) to better understand their perspective if they think the transmute is correct.

---

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: Re-remove [`transmute_undefined_repr`] from default set of enabled lints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants