Skip to content

resolver: don't use Finalize when resolving visibilities during AST expansion#158689

Open
LorrensP-2158466 wants to merge 1 commit into
rust-lang:mainfrom
LorrensP-2158466:non-final-vis-resolve
Open

resolver: don't use Finalize when resolving visibilities during AST expansion#158689
LorrensP-2158466 wants to merge 1 commit into
rust-lang:mainfrom
LorrensP-2158466:non-final-vis-resolve

Conversation

@LorrensP-2158466

Copy link
Copy Markdown
Contributor

During AST expansion we need to resolve visibilities of items. It uses Finalize to make sure to get a definite result on the resolution (a sort of hacky way). this pr disables the use of Finalize as it contradicts the invariant proposed by it:

/// Invariant: if `Finalize` is used, expansion and import resolution must be complete.
struct Finalize { ... }

@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 Jul 2, 2026
@LorrensP-2158466

Copy link
Copy Markdown
Contributor Author

lets see what CI says.

@petrochenkov petrochenkov self-assigned this Jul 2, 2026
@LorrensP-2158466 LorrensP-2158466 marked this pull request as ready for review July 2, 2026 14:26
@rustbot rustbot 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 Jul 2, 2026
@LorrensP-2158466

Copy link
Copy Markdown
Contributor Author

This should probably have a crater check run, right?

@petrochenkov

Copy link
Copy Markdown
Contributor

Could you add the "finalize implies !has_unexpanded_invocations" asserts that we discussed in the previous PR (to both globs and non-globs)?

@petrochenkov

Copy link
Copy Markdown
Contributor

This should probably have a crater check run, right?

Beta run should be enough, if this didn't break anything in the test suite, then I'm 99% sure it won't break anything in practice.

@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 Jul 2, 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.

3 participants