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

Skip pub structs with repr(c) and repr(transparent) in dead code analysis #127104

Closed
wants to merge 1 commit into from

Conversation

mu001999
Copy link
Contributor

Fixes #126169

@rustbot
Copy link
Collaborator

rustbot commented Jun 29, 2024

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 29, 2024
@mu001999
Copy link
Contributor Author

r? @compiler-errors

@compiler-errors
Copy link
Member

I think any changes to lints should go through T-lang. Sorry for taking long to review this.

@compiler-errors compiler-errors added T-lang Relevant to the language team, which will review and decide on the PR/issue. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). I-lang-nominated Nominated for discussion during a lang team meeting. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2024
@tgross35
Copy link
Contributor

This came up on Zulip https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/Status.20of.20fix.20for.20dead_code.20warnings.20on.20repr.28C.29.20structs. It seems rather unfortunate that large bindings crates are needing to allow(dead_code) globally in order to work around this (libc, windows).

If this proposed solution is decided not to be acceptable, unfortunately we should probably consider temporarily reverting or adjusting part of #125572. One suggestion is to emit a new lint within dead_code that can be disabled separately, rather than allow(dead_code) being the only way to opt out #126169 (comment)

Unfortunately this behavior just branched, so:

@rustbot label +beta-nominated

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 21, 2024
@workingjubilee
Copy link
Member

This PR has been introduced to address a diagnostics regression. The original PR that introduced the behavior this change was meant to prevent regressing can no longer be cleanly reverted on beta. This is thus now the most minimal diff if we want to apply the change to beta, even if it means we introduce new code.

@bors
Copy link
Contributor

bors commented Jul 26, 2024

☔ The latest upstream changes (presumably #128222) made this pull request unmergeable. Please resolve the merge conflicts.

@compiler-errors
Copy link
Member

See #128329 (comment)

I think that all of this new dead code analysis landed recently should probably be reverted and reworked, since then we're not pressured to put up hacky fixes in favor of getting it correct and well documented the first time 👍

@bors
Copy link
Contributor

bors commented Aug 3, 2024

☔ The latest upstream changes (presumably #128404) made this pull request unmergeable. Please resolve the merge conflicts.

@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 2, 2024
@Mark-Simulacrum
Copy link
Member

Dropping beta-nominated, looks like the original PR was reverted on beta 1.81 (via #128606).

@mu001999 mu001999 closed this Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-lang-nominated Nominated for discussion during a lang team meeting. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dead_code lint wrongly warns about public repr(C) structs with private fields but no constructors
8 participants