Skip to content

Add a lot more db lifetimes - #20897

Closed
smmalis37 wants to merge 1 commit into
rust-lang:masterfrom
smmalis37:more-db
Closed

smmalis37 wants to merge 1 commit into
rust-lang:masterfrom
smmalis37:more-db

Conversation

@smmalis37

@smmalis37 smmalis37 commented Oct 24, 2025

Copy link
Copy Markdown
Contributor

As per the request on #20896, I removed no_lifetime and added a 'db to InternedOpaqueTyId. This propagated way farther than I expected, but I followed them all. The macro tweaks were the annoying part, as I had to work around the differences between declaring a type in a signature and calling a method on the type. But it all seems to work.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 24, 2025
Ctor
for SolverDefId
);
impl<'db> From<AdtId> for SolverDefId<'db> {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

impl_from! doesn't work with generics unfortunately.

let method = quote! {
#sig {
#wrapper_struct::new(self, #interned_pat)
#type_constructor new(self, #interned_pat)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Previously, when given the type InternedOpaqueTyId<'db> this would generate InternedOpaqueTyId<'db>::new, which is invalid syntax. Now it removes all the generics, leaving them to be inferred, producing just InternedOpaqueTyId::new. The same applies below.


self.signature = parse_quote!(
fn #ident(&self, id: #interned_key) -> #interned_return_ty
fn #ident #generics(#receiver, id: #interned_key) -> #interned_return_ty

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This &self needs the lifetime now too, hence this change.

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

I know I've asked for this, but after seeing the diff I'm not sure I prefer that 😅

rustc_type_ir::solve::ExternalConstraintsData<DbInterner<'db>>;

interned_vec_nolifetime_salsa!(SolverDefIds, SolverDefId);
interned_vec_nolifetime_salsa!(SolverDefIds, SolverDefId<'db>);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does this need more lifetime propagation too? The macro is named nolifetime but it seems to work fine despite the lifetime...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I guess other uses of this macro already had lifetimes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think its about the declaration it creates, not the things it consumes

@Veykril

Veykril commented Dec 7, 2025

Copy link
Copy Markdown
Member

Would love to get the ball rolling for the lifetime changes, could you rebase this if you get the time? Happy to merge it then

@rustbot

rustbot commented Mar 12, 2026

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (possibly #21804) made this pull request unmergeable. Please resolve the merge conflicts.

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

We didn't get OP's response for some months and it's going to be pretty hard to rebase this now. Closing.

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 23, 2026
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.

4 participants