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

Only determine impl-trait desugaring kind once #96529

Open
5 tasks
cjgillot opened this issue Apr 28, 2022 · 3 comments
Open
5 tasks

Only determine impl-trait desugaring kind once #96529

cjgillot opened this issue Apr 28, 2022 · 3 comments
Assignees
Labels
A-ast Area: AST A-HIR Area: The high-level intermediate representation (HIR) C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cjgillot
Copy link
Contributor

cjgillot commented Apr 28, 2022

impl-trait can either be desugared as a generic parameter (universal), as an opaque type (existential), or forbidden. This desugaring is currently on the AST in two places: def_collector and ast_lowering. The two logic are not consistent with one another. When this happens, ast_lowering is right.

dyn Trait<T: Bound> sometimes desugars to dyn Trait<T = impl Bound>, and sometimes does not.

We should only determine the desugaring once. This can be achieved by:

  • saving the desugaring from def_collector into an impl_trait_context: FxHashMap<LocalDefId, ImplTraitContext> in Resolver;
  • add impl_trait_id: NodeId in rustc_ast::AssocConstraint and use it to create the impl-trait's LocalDefId;
  • introduce UniversalInDyn to handle the dyn Trait<T: Bound> case in def_collector and create the definition there, desugar_to_impl_trait should be replaced by resolver.opt_local_def_id(constraint.impl_trait_id).is_some();
  • move the &mut Vec in rustc_ast_lowering::ImplTraitContext to LoweringContext, the new field should be saved and restored by with_hir_id_owner;
  • merge rustc_ast_lowering::ImplTraitContext and rustc_resolve::ImplTraitContext, introducing a ResolverAstLowering::get_impl_trait_context(LocalDefId) -> Option<ImplTraitContext> to access the information.

Please reach out on Zulip for any question.

@cjgillot cjgillot added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. A-HIR Area: The high-level intermediate representation (HIR) E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels Apr 28, 2022
@randomicon00
Copy link
Contributor

@rustbot claim

bors added a commit to rust-lang-ci/rust that referenced this issue May 20, 2022
…ochenkov

Lint single-use lifetimes during AST resolution

This PR rewrites `single_use_lifetime` and `unused_lifetime` lints to be based on the AST.
We have more information at our disposal, so we can reduce the amount of false positives.

Remaining false positive: single-use lifetimes in argument-position impl-trait.
I'm waiting for rust-lang#96529 to be fixed to have a clean and proper solution here.

Closes rust-lang#54079
Closes rust-lang#55057
Closes rust-lang#55058
Closes rust-lang#60554
Closes rust-lang#69952

r? `@petrochenkov`
@Enselic Enselic added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-cleanup Category: PRs that clean code up or issues documenting cleanup. labels Mar 16, 2024
@vE5li
Copy link

vE5li commented May 29, 2024

Is anyone working on this? If not I would like to take a look

@randomicon00
Copy link
Contributor

randomicon00 commented May 29, 2024

The tasks are interlinked, from my understanding, and the version that I have has one, two, and three almost done and I stopped at the last item. In fact, I am stuck there, give me like a week or two so I refresh my memory and get an exact idea about what's blocking me on that last item?

@workingjubilee workingjubilee added the A-ast Area: AST label Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ast Area: AST A-HIR Area: The high-level intermediate representation (HIR) C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants