Skip to content

Fix order-dependent visibility diagnostics#155948

Open
SynapLink wants to merge 1 commit intorust-lang:mainfrom
SynapLink:fix/pub-visibility-order
Open

Fix order-dependent visibility diagnostics#155948
SynapLink wants to merge 1 commit intorust-lang:mainfrom
SynapLink:fix/pub-visibility-order

Conversation

@SynapLink
Copy link
Copy Markdown
Contributor

@SynapLink SynapLink commented Apr 28, 2026

Fixes #40066.
Fixes #109657.
Delay visibility path diagnostics until module collection has finished, so paths to later non-ancestor modules report E0742 instead of an unresolved path error.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 28, 2026
@rust-log-analyzer

This comment has been minimized.

@SynapLink SynapLink force-pushed the fix/pub-visibility-order branch from bfb4b8a to 21b1458 Compare April 28, 2026 23:08
@SynapLink SynapLink marked this pull request as ready for review April 29, 2026 00:19
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 29, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 29, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 29, 2026

r? @mu001999

rustbot has assigned @mu001999.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 73 candidates
  • Random selection from 20 candidates

@petrochenkov petrochenkov self-assigned this Apr 29, 2026
match self.try_resolve_visibility(vis, true) {
Ok(vis) => vis,
Err(err) => {
if !self.delay_vis_resolution(vis) {
Copy link
Copy Markdown
Contributor

@petrochenkov petrochenkov Apr 29, 2026

Choose a reason for hiding this comment

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

I think it would be simpler to delay unconditionally.
DelayedVisResolutionError can contain the actual VisResolutionError, if late resolve_path fails then we report the new error, if it succeeds then we report the stashed error.

View changes since the review


fn delay_vis_resolution(&mut self, vis: &ast::Visibility) -> bool {
let ast::VisibilityKind::Restricted { ref path, id, .. } = vis.kind else {
return false;
Copy link
Copy Markdown
Contributor

@petrochenkov petrochenkov Apr 29, 2026

Choose a reason for hiding this comment

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

This case is unreachable, non-Restricted visibilities cannot produce errors.

View changes since the review

let ast::VisibilityKind::Restricted { ref path, id, .. } = vis.kind else {
return false;
};
let Ok(segments) = self.visibility_path_segments(path) else {
Copy link
Copy Markdown
Contributor

@petrochenkov petrochenkov Apr 29, 2026

Choose a reason for hiding this comment

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

We can probably just save Box<Path> in the delayed error, without converting it into segments.

View changes since the review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Or perhaps we can clone the whole ast::Visibility to store it in the delayed error, and then call try_resolve_visibility from report_delayed_vis_resolution_errors instead of reusing just parts of it.

),
})
}
ast::VisibilityKind::Restricted { ref path, id, .. } => {
Copy link
Copy Markdown
Contributor

@petrochenkov petrochenkov Apr 29, 2026

Choose a reason for hiding this comment

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

Could you extract the logic below and share it between report_delayed_vis_resolution_errors and this function?

View changes since the review

@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 Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

unexpected behavior of pub(in other_module) Error on bad pub(path) depends on module ordering

5 participants