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.

Sign up for GitHub

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

Jump to bottom

Resolve enum field visibility correctly #79956

Merged
merged 1 commit into from
Dec 13, 2020
Merged

Resolve enum field visibility correctly #79956

merged 1 commit into from
Dec 13, 2020

Conversation

camelid
Copy link
Member

@camelid camelid commented Dec 12, 2020

Fixes #79593. 🎉

Previously, this code treated enum fields' visibility as if they were
struct fields. However, that's not correct because the visibility of a
struct field with ast::VisibilityKind::Inherited is private to the
module it's defined in, whereas the visibility of an enum field with
ast::VisibilityKind::Inherited is the visibility of the enum it
belongs to.

@camelid camelid added A-resolve Area: Path resolution A-visibility Area: Visibility / privacy. labels Dec 12, 2020
@rust-highfive
Copy link
Collaborator

r? @eddyb

(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 Dec 12, 2020
@jyn514 jyn514 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 12, 2020
@jyn514
Copy link
Member

jyn514 commented Dec 12, 2020

r? @petrochenkov

@rust-highfive rust-highfive assigned petrochenkov and unassigned eddyb Dec 12, 2020
camelid
camelid commented Dec 12, 2020
View reviewed changes
src/test/ui/issues/issue-79593.stderr
LL | foo::Pub {};
| ^^^^^^^^

error[E0063]: missing field `y` in initializer of `Enum`
Copy link
Member Author

Choose a reason for hiding this comment

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

This should probably print

error[E0063]: missing field `y` in initializer of `Enum::Variant`

but that's unrelated to this change.

(I'm leaving this as a todo for myself.)

petrochenkov
petrochenkov reviewed Dec 12, 2020
View reviewed changes
compiler/rustc_resolve/src/build_reduced_graph.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/lib.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/build_reduced_graph.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/build_reduced_graph.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/build_reduced_graph.rs Outdated Show resolved Hide resolved
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2020
@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 12, 2020
@petrochenkov
Copy link
Contributor

Could you also squashing commits after addressing the comments?

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2020
@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 12, 2020
@camelid
Copy link
Member Author

camelid commented Dec 12, 2020

I believe I have addressed all of your comments now.

@petrochenkov
Copy link
Contributor

Thanks!
r=me after squashing commits.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2020
@camelid
Resolve enum field visibility correctly
5ce3f4c 
Previously, this code treated enum fields' visibility as if they were
struct fields. However, that's not correct because the visibility of a
struct field with `ast::VisibilityKind::Inherited` is private to the
module it's defined in, whereas the visibility of an *enum* field with
`ast::VisibilityKind::Inherited` is the visibility of the enum it
belongs to.
@camelid
Copy link
Member Author

camelid commented Dec 12, 2020

Squashed!

@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 12, 2020
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 12, 2020

📌 Commit 5ce3f4c has been approved by petrochenkov

@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 Dec 12, 2020
@bors
Copy link
Contributor

bors commented Dec 13, 2020

⌛ Testing commit 5ce3f4c with merge d149b65...

@bors
Copy link
Contributor

bors commented Dec 13, 2020

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing d149b65 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 13, 2020
@bors bors merged commit d149b65 into rust-lang:master Dec 13, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 13, 2020
@camelid camelid deleted the variant-field-vis branch December 13, 2020 18:08
@camelid camelid added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 13, 2020
@camelid
Copy link
Member Author

camelid commented Dec 13, 2020

Nominating for beta-backport since this error comes up fairly frequently and the unfixed version is very confusing and annoying, and it's likely that it's even more so for newcomers. (The issue this fixed was P-high because of the reasons I listed.)

@spastorino
Copy link
Member

discussed in T-compiler meeting, declined to backport.

@rustbot modify labels: -beta-nominated

@rustbot rustbot removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 17, 2020
JohnTitor added a commit to JohnTitor/rust-semverver that referenced this pull request Mar 31, 2021
@JohnTitor
Update to nightly-2020-12-14
d68d47f 
rust-lang/rust#79956 improved enum's visibility and now we can check
the visibility more precisely.
JohnTitor added a commit to JohnTitor/rust-semverver that referenced this pull request Mar 31, 2021
@JohnTitor
Update to nightly-2020-12-14
714fb0d 
rust-lang/rust#79956 improved enum's visibility and now we can check
the visibility more precisely.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

@jyn514 jyn514 jyn514 left review comments

Assignees

@petrochenkov petrochenkov

Labels
A-resolve Area: Path resolution A-visibility Area: Visibility / privacy. merged-by-bors This PR was explicitly merged by bors. 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
Milestone
1.50.0
Development

Successfully merging this pull request may close these issues.

Diagnostic regression (1.47 -> 1.48): public enum fields incorrectly described as "inaccessible fields".
8 participants
@camelid @rust-highfive @jyn514 @petrochenkov @bors @spastorino @eddyb @rustbot