Skip to content

feat: make trait assoc items become inactive due to cfg #12965

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

Merged
merged 6 commits into from
Aug 22, 2022

Conversation

kartva
Copy link
Contributor

@kartva kartva commented Aug 7, 2022

fixes #12394

@kartva
Copy link
Contributor Author

kartva commented Aug 7, 2022

Narrowed down the logic for ignoring the assoc items to

fn collect(&mut self, item_tree: &ItemTree, tree_id: TreeId, assoc_items: &[AssocItem]) {
let container = self.container;
self.items.reserve(assoc_items.len());
'items: for &item in assoc_items {
let attrs = item_tree.attrs(self.db, self.module_id.krate, ModItem::from(item).into());
if !attrs.is_cfg_enabled(self.expander.cfg_options()) {
continue;
}

@kartva
Copy link
Contributor Author

kartva commented Aug 7, 2022

Searching for is_cfg_enabled is the way to go for finding places where things are filtered without emitting a diagnostic.

@kartva
Copy link
Contributor Author

kartva commented Aug 8, 2022

@flodiebold
For the plumbing you mentioned, most sub-items are filtered in hir-def/src/adt, while inactive diagnostics are emitted in hir-def/src/nameres/collector (for inactive module items). Additionally, the diagnostics structs are only available to modules in the nameres module. I saw #8973 which moved diagnostics to its current position.

Should I try to thread a diagnostics vec for all the functions that filter cfg'd fields? Is there a particular reason why fields are filtered at the AST stage, instead of the HIR stage like module items?

@kartva kartva marked this pull request as ready for review August 20, 2022 08:02
Copy link
Contributor Author

@kartva kartva left a comment

Choose a reason for hiding this comment

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

@kartva kartva changed the title fix trait assoc item not becoming inactive due to cfg feat: make trait assoc items become inactive due to cfg Aug 20, 2022
@Veykril
Copy link
Member

Veykril commented Aug 22, 2022

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Aug 22, 2022

📌 Commit 23c00ed has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 22, 2022

⌛ Testing commit 23c00ed with merge dea1639...

@bors
Copy link
Contributor

bors commented Aug 22, 2022

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing dea1639 to master...

@bors bors merged commit dea1639 into rust-lang:master Aug 22, 2022
@lnicola
Copy link
Member

lnicola commented Aug 29, 2022

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Associated methods removed by conditional compilation are not dimmed
4 participants