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

Suggest making private tuple struct field public #106579

Merged
merged 3 commits into from
Jan 12, 2023

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jan 8, 2023

Fix #52144.

@rustbot
Copy link
Collaborator

rustbot commented Jan 8, 2023

r? @Nilstrieb

(rustbot has picked a reviewer for you, use r? to override)

@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 Jan 8, 2023
Comment on lines +884 to +887
/// Span of the privacy modifier in fields of an item `DefId` accessible with dot syntax.
/// Used for hints during error reporting.
field_visibility_spans: FxHashMap<DefId, Vec<Span>>,

Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to extend and reuse the field_names field for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

field_names only had the Symbol and I wasn't sure if there was any place that relied on it being always a Vec, causing perf impact if needing to map/collect it every time. But yes, we could do that :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only difference I see is that this approach will only store the privacy span vec for tuple structs and nothing else.

@estebank estebank force-pushed the issue-52144 branch 2 times, most recently from a6c6886 to 9369bfa Compare January 10, 2023 20:58
* Account for `struct S(pub(super)Ty);` in suggestion
* Suggest changing field visibility in E0603 too
@Noratrieb
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jan 12, 2023

📌 Commit 41e66d9 has been approved by Nilstrieb

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 Jan 12, 2023
Noratrieb added a commit to Noratrieb/rust that referenced this pull request Jan 12, 2023
Suggest making private tuple struct field public

Fix rust-lang#52144.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 12, 2023
Rollup of 6 pull requests

Successful merges:

 - rust-lang#105806 (Support eager subdiagnostics again)
 - rust-lang#106322 (Handle inference variables in `CollectAllMismatches` correctly)
 - rust-lang#106579 (Suggest making private tuple struct field public)
 - rust-lang#106714 (remove unreachable error code `E0490`)
 - rust-lang#106751 (Fix rendering 'const' in header for intrinsics)
 - rust-lang#106761 (Add `WaffleLapkin` to compiler reviewers)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 25fd633 into rust-lang:master Jan 12, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 12, 2023
@estebank estebank deleted the issue-52144 branch November 9, 2023 05:13
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.

Non-public tuple fields causes confusing error message when crossing module boundaries
5 participants