Skip to content

Conversation

@Veykril
Copy link
Member

@Veykril Veykril commented Aug 16, 2025

No description provided.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 16, 2025
@ChayimFriedman2
Copy link
Contributor

I still think we should use our own attaching, not Salsa's.

@Veykril
Copy link
Member Author

Veykril commented Aug 17, 2025

I still don't see the benefit in that, but I don't mind either way. Note that if we do all of these calls here are required for the new thing as well though

@ChayimFriedman2
Copy link
Contributor

Unless we allow it to be reentrant.

@Veykril
Copy link
Member Author

Veykril commented Aug 17, 2025

That has nothing to do with this, any assist functionality might calll into the trait solver stuff without going through query. So we need to make sure to setup the tls before hand like this PR does. Using a separate tls doesn't change that.

@ChayimFriedman2
Copy link
Contributor

What I meant is that we don't need to not attach it in highlighting.

@ShoyuVanilla
Copy link
Member

ShoyuVanilla commented Aug 18, 2025

I guess this would fix the new panic in our release workflow here?

#20443 (comment)

pub fn inner(&self) -> &WithCachedTypeInfo<TyKind<'db>> {
salsa::with_attached_database(|db| {
let inner = &self.kind_(db).0;
// SAFETY: The caller already has access to a `Ty<'db>`, so borrowchecking will
// make sure that our returned value is valid for the lifetime `'db`.
unsafe { std::mem::transmute(inner) }
})
.unwrap()
}

Though I don't have strong opinions on design choices but I'm +1 on merging this to ship our first next solver unstable release

@Veykril
Copy link
Member Author

Veykril commented Aug 18, 2025

It would fix it yes

@Veykril Veykril added this pull request to the merge queue Aug 18, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 18, 2025
@Veykril Veykril force-pushed the veykril/push-wppxsntzqtou branch from 016ee57 to aed0fec Compare August 18, 2025 07:54
@Veykril Veykril closed this Aug 18, 2025
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 18, 2025
@Veykril Veykril reopened this Aug 18, 2025
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 18, 2025
@Veykril Veykril enabled auto-merge August 18, 2025 07:55
@Veykril Veykril added this pull request to the merge queue Aug 18, 2025
Merged via the queue into master with commit a905e3b Aug 18, 2025
20 of 30 checks passed
@Veykril Veykril deleted the veykril/push-wppxsntzqtou branch August 18, 2025 08:33
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 18, 2025
@lnicola
Copy link
Member

lnicola commented Oct 14, 2025

changelog fixup #20329

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.

6 participants