Skip to content

fix(hir): Do not ignore the name in PartialEq for Type::NamedGeneric for implicit unbound#11351

Merged
aakoshh merged 2 commits intomasterfrom
af/11347-type-eq-unbound-check-name
Jan 27, 2026
Merged

fix(hir): Do not ignore the name in PartialEq for Type::NamedGeneric for implicit unbound#11351
aakoshh merged 2 commits intomasterfrom
af/11347-type-eq-unbound-check-name

Conversation

@aakoshh
Copy link
Copy Markdown
Contributor

@aakoshh aakoshh commented Jan 27, 2026

Description

Problem

Resolves #11347

Summary

Changes the PartialEq implementation for Type to compare the name of NamedGenerics when both are unbound and implicitly added.

Additional Context

I read the explanation in #8184 and seemed to revolve around creating named generics for the ordered generics of a trait that appears in the type bound. I'm not sure if by considering implicitly added variables different we will be rejecting something that should compile, but as it is today, it presents a soundness bug, where there are no comparisons on two generics, so we can pass invalid data to functions as long as they expect generics and the source is generic as well. Being a bit more conservative doesn't seem to hurt any tests.

Note that this builds on #11346 where the named generics were changed to be named e.g. <A as Eggs>::Item, but without that PR their name would just be Item, in which case the test I added here would still consider them equivalent to any other type with the same name.

Also noting that the ticket showcases ordered generics, but here we have associated types.

User Documentation

Check one:

  • No user documentation needed.
  • Changes in docs/ included in this PR.
  • [For Experimental Features] Changes in docs/ to be submitted in a separate PR.

PR Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@aakoshh aakoshh changed the base branch from master to af/10864-assoc-and-gen-same-name January 27, 2026 16:39
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 897b83e Previous: 8592c53 Ratio
test_report_zkpassport_noir_rsa_ 1 s 0 s +∞

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Brillig Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 897b83e Previous: 8592c53 Ratio
rollup-checkpoint-merge 0.002 s 0.001 s 2

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Base automatically changed from af/10864-assoc-and-gen-same-name to master January 27, 2026 17:10
@aakoshh aakoshh force-pushed the af/11347-type-eq-unbound-check-name branch from 32059a4 to f3bcd40 Compare January 27, 2026 17:18
@aakoshh aakoshh enabled auto-merge January 27, 2026 17:20
@aakoshh aakoshh requested a review from a team January 27, 2026 17:23
@jfecher
Copy link
Copy Markdown
Contributor

jfecher commented Jan 27, 2026

We should likely remove this equality check when checking traits entirely eventually but it doesn't need to be this PR

@aakoshh aakoshh added this pull request to the merge queue Jan 27, 2026
Merged via the queue into master with commit 4a862a2 Jan 27, 2026
136 checks passed
@aakoshh aakoshh deleted the af/11347-type-eq-unbound-check-name branch January 27, 2026 17:56
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.

Mixup of associated types across different traits not rejected

2 participants