Skip to content

internal: Do not the same types twice in hir-ty lowering - #23196

Open
ChayimFriedman2 wants to merge 2 commits into
rust-lang:masterfrom
ChayimFriedman2:no-relower
Open

internal: Do not the same types twice in hir-ty lowering#23196
ChayimFriedman2 wants to merge 2 commits into
rust-lang:masterfrom
ChayimFriedman2:no-relower

Conversation

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

Again for the intent of making AnonConst a tracked struct (as well as for perf).

We still have one problematic query: resolve_type_param_assoc_type_shorthand(). It is problematic and I need to think what to do with it.

And a drive-by fix in the second commit.

Best reviewed commit by commit. The commits are fully separate.

Again for the intent of making `AnonConst` a tracked struct (as well as for perf).

We still have one problematic query: `resolve_type_param_assoc_type_shorthand()`. It is problematic and I need to think what to do with it.
E.g.:
```rust
trait Trait
where
    Self::Assoc: OtherTrait,
{
    type Assoc;
}
```
Is equivalent to:
```rust
trait Trait {
    type Assoc: OtherTrait;
}
```

The code was copied from rustc and adapted.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2026
Comment on lines +275 to +281
impl<'db> Default for Clauses<'db> {
#[inline]
fn default() -> Self {
Clauses::empty(DbInterner::conjure())
}
}

@Veykril Veykril Sep 6, 2026

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.

Do we have to add this? I'd prefer not to ideally conjure shouldn't exist in the first place, so I'd rather remove uses of it than add them

View changes since the review

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 don't like conjure() too much but I wouldn't want to complicate the API just to avoid it.

Alternatively, we don't really need the database in order to intern and only do that for API reasons. We can avoid that and probably get rid of conjure() completely.

pub fn get(&self, local_id: LocalTypeOrConstParamId) -> Ty<'_> {
match self.map.get(local_id) {
Some(ty) => ty.as_ref(),
None => Ty::new_error(DbInterner::conjure(), ErrorGuaranteed),

@Veykril Veykril Sep 6, 2026

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.

likewise threading the database through here shouldn't be much work

View changes since the review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants