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

fix: implemeted lifetime transformation fot assits #14875

Conversation

ponyii
Copy link
Contributor

@ponyii ponyii commented May 22, 2023

A part of #13363
I expect to implement transformation of const params in a separate PR

Other assists and a completion affected:

  • generate_function currently just ignores lifetimes and, consequently, is not affected
  • inline_call and replace_derive_with... don't seem to need lifetime transformation
  • trait_impl (a completion) is fixed and tested

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 22, 2023
.into_iter()
.flat_map(|it| it.lifetime_params(db))
.zip(self.substs.lifetimes.clone())
.filter_map(|(k, v)| Some((k.name(db).to_string(), v.lifetime()?)))
Copy link
Contributor Author

@ponyii ponyii May 23, 2023

Choose a reason for hiding this comment

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

todo: lifetime resolver is already written, could I reuse it somehow?

upd: I believe I shouldn't.
basically this resolver traverses through a ast::Lifetime's ancestors looking for one with the same name, then converts the found ast::LifetimeParam to hir::LifetimeParam. in our case, there's no need neither in conversion nor in the search, as all the lifetimes to be changed are generic params of a known ancestor, the source trait.
(but I don't really understand why SemanticsScope can't resolve a lifetime)

self.lifetime_params(db).into_iter().chain(ty_params).collect()
}

pub fn lifetime_params(self, db: &dyn HirDatabase) -> Vec<GenericParam> {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub fn lifetime_params(self, db: &dyn HirDatabase) -> Vec<GenericParam> {
pub fn lifetime_params(self, db: &dyn HirDatabase) -> Vec<LifetimeParam> {

@@ -9,6 +9,14 @@ use syntax::{
ted, SyntaxNode,
};

#[derive(Default)]
struct Substs {
Copy link
Member

Choose a reason for hiding this comment

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

nit, let's rename this to AstSubsts as we might have Substs in the hir layer in the future

@Veykril Veykril 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 May 28, 2023
@ponyii ponyii force-pushed the fix/implement-missing-members-do-not-transform-lifetimes branch from ba6e7ce to 7f45ccc Compare June 3, 2023 17:35
@ponyii ponyii requested a review from Veykril June 3, 2023 17:52
@Veykril
Copy link
Member

Veykril commented Jun 10, 2023

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Jun 10, 2023

📌 Commit 7f45ccc has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 10, 2023

⌛ Testing commit 7f45ccc with merge 95228d2...

@bors
Copy link
Collaborator

bors commented Jun 10, 2023

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

@bors bors merged commit 95228d2 into rust-lang:master Jun 10, 2023
10 checks passed
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants