fix: Check NamedGenerics by id, not name

[jfecher]

Description

Problem

Inspired by #11346 (comment) (but still requires the fixes from that PR)
Resolves #11358

Summary

Changes type checking to stop relying on the names for NamedGenerics, which may be the same (at least, pre-Akosh's PRs) in some cases with different traits which each define associated types of the same name. Comparing integers should be faster than comparing strings anyway.

Additional Context

Waiting to merge this until after @aakoshh's PRs to use the tests added there

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.

[github-actions]

⚠️ 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: fe17e50	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

[github-actions]

⚠️ 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: 02b2f90	Previous: ab1afd5	Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr	199 s	148 s	1.34

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

CC: @TomAFrench

[aakoshh]

Could you add an explanation in the Additional Context about why we compared names in the first place?

[aakoshh]

This should resolve #11358 right?

[jfecher]

@aakoshh

Could you add an explanation in the Additional Context about why we compared names in the first place?

IIRC, way back when NamedGenerics simply never had a TypeVariable member as they were never meant to be bound over. Then this was just never changed until now.

This should resolve #11358 right?

You'd think so, but no. I'm not sure why, but that's why I think there must be some odd type variable binding behavior or similar going on.

[aakoshh]

You'd think so, but no. I'm not sure why, but that's why I think there must be some odd type variable binding behavior or similar going on.

Ah, this is what I saw here: #11348 (comment)

[jfecher]

Ah, this is what I saw here: #11348 (comment)

Same behavior, but I found why this was happening and just pushed a commit to fix it. In try_unify_with_flags we had a raw equality check on types which for NamedGenerics meant we were still falling back to the old name1 == name2 behavior and early returning if so.

[github-actions]

⚠️ Performance Alert ⚠️

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

Benchmark suite	Current: 02b2f90	Previous: ab1afd5	Ratio
perfectly_parallel_batch_inversion_opcodes	2801102 ns/iter (± 2006)	2267576 ns/iter (± 1479)	1.24

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

CC: @TomAFrench

[github-actions]

⚠️ Performance Alert ⚠️

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

Benchmark suite	Current: 02b2f90	Previous: ab1afd5	Ratio
rollup-block-root	0.005 s	0.004 s	1.25

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

CC: @TomAFrench

[aakoshh]

Nice!

I was wondering if we could get more descriptive errors based on TypeMismatchWithSource but it doesn't look like it.

I'll have a look at appending the path to the type.