Skip to content

Conversation

l4l
Copy link
Contributor

@l4l l4l commented Oct 21, 2022

Resolves #103317

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 21, 2022
@rust-highfive
Copy link
Contributor

r? @wesleywiser

(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 Oct 21, 2022
@wesleywiser
Copy link
Member

r? compiler

@rustbot rustbot assigned nagisa and unassigned wesleywiser Oct 27, 2022
@nagisa
Copy link
Member

nagisa commented Oct 28, 2022

Doing a quick first pass, will take a closer look later this week.

Would it make sense to also add a run-rustfix test for this, given that the issue mentions cargo fix doing wrong replacements?

I would even consider adding a test without #[allow(unused)], as AFAICT it doesn’t actually impact the scenario this is testing for (took me a moment to understand this.)

@l4l
Copy link
Contributor Author

l4l commented Oct 28, 2022

Ooh, there are tests for rustfix, that's really nice, will add one.

I would even consider adding a test without #[allow(unused)]

You mean adding an additional one for run-rustfix, right? There are a lot of warnings for cargo-check (check-pass).

@l4l
Copy link
Contributor Author

l4l commented Oct 29, 2022

Added .fixed file for the same test and put allow(unused) closer to the enum which seems to be clearer.

@nagisa
Copy link
Member

nagisa commented Oct 30, 2022

You mean adding an additional one for run-rustfix, right? There are a lot of warnings for cargo-check (check-pass).

Yeah, but upon further review I don’t believe that’s necessary at all.

@bors r+ Thanks!

(Side note: it is quite unfortunate that the lint callback infrastructure requires us to figure if a field belongs to an enum variant after the fact, rather than allowing to just change this entire visitor to skip walking enums in the first place. Welp.)

@bors
Copy link
Collaborator

bors commented Oct 30, 2022

📌 Commit 137271a has been approved by nagisa

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 30, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 31, 2022
Rollup of 5 pull requests

Successful merges:

 - rust-lang#103338 (Fix unreachable_pub suggestion for enum with fields)
 - rust-lang#103603 (Lang item cleanups)
 - rust-lang#103732 (Revert "Make the `c` feature for `compiler-builtins` opt-in instead of inferred")
 - rust-lang#103766 (Add tracking issue to `error_in_core`)
 - rust-lang#103789 (Update E0382.md)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5ee0fb1 into rust-lang:master Oct 31, 2022
@rustbot rustbot added this to the 1.67.0 milestone Oct 31, 2022
@l4l l4l deleted the enum-unreachable-pub branch October 31, 2022 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid unreachable_pub suggestion for enum with fields
6 participants