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

Iterate over maybe_unused_trait_imports when checking dead trait imports #96873

Closed
cjgillot opened this issue May 9, 2022 · 4 comments · Fixed by #97609
Closed

Iterate over maybe_unused_trait_imports when checking dead trait imports #96873

cjgillot opened this issue May 9, 2022 · 4 comments · Fixed by #97609
Assignees
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@cjgillot
Copy link
Contributor

cjgillot commented May 9, 2022

The resolver gathers a list of possibly unused trait imports in maybe_unused_trait_imports. This list is passed to resolver outputs, into the maybe_unused_trait_import query. This query takes a LocalDefId and returns whether the import is possibly unused.

This is used by the post-typeck unused import check in rustc_typeck::check_unused. This query iterates over all the use statements in the crate, checks whether they may be unused trait imports, and warns if they are indeed unused.

As there are typically very few unused trait imports compared to the number of items in a crate, it would make sense to directly iterate on maybe_unused_trait_imports.

Steps:

  • make maybe_unused_trait_imports an FxIndexSet both in Resolver and ResolverOutputs, this will ensure a deterministic iteration order;
  • rename maybe_unused_trait_import query and give it the signature maybe_unused_trait_imports: () -> FxIndexSet<LocalDefId>, and adapt the call site; we do not want to access tcx.resolutions(()) directly, as this would prevent the query system from detecting that the set has not changed;
  • in rustc_typeck::check_unused::check_crate, inline check_import and replace the loop over tcx.hir().items by a loop over tcx.maybe_unused_trait_imports(());
  • the filter over tcx.def_kind there can be removed too (or replaced by a debug-assertion): the set may only point to use statements.

Extra:

  • investigate why we check for item.span.is_empty(), and the consequences of removing that check;
  • investigate the impact on perf of converting the FxIndexSet to a simple Vec when producing ResolverOutputs from Resolver.

I am available on Zulip for any question.

@cjgillot cjgillot added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-help-wanted Call for participation: Help is requested to fix this issue. labels May 9, 2022
@Elliot-Roberts
Copy link
Contributor

@rustbot claim

@lionellloh
Copy link
Contributor

@Elliot-Roberts are you still working on this?

@Elliot-Roberts
Copy link
Contributor

Hello yes sorry for the super slow pace. I am still working on it (mostly reading the rustc dev guide so far). Is that alright or is it important to get this done quickly?

@lionellloh
Copy link
Contributor

@Elliot-Roberts no worries, I am a beginner looking for E-easy tasks, hence the qn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants