fix: Check for overlapping implementations #11328
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ 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: 31ae5a1 | Previous: 84f4b3a | Ratio |
|---|---|---|---|
perfectly_parallel_batch_inversion_opcodes |
2794536 ns/iter (± 18304) |
2268790 ns/iter (± 6112) |
1.23 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
There was a problem hiding this comment.
⚠️ 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: 2f059c0 | Previous: f00ead9 | Ratio |
|---|---|---|---|
test_report_zkpassport_noir-ecdsa_ |
3 s |
1 s |
3 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
There was a problem hiding this comment.
⚠️ 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: 65cc774 | Previous: c695737 | Ratio |
|---|---|---|---|
sha512-100-bytes |
0.07 s |
0.056 s |
1.25 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
| for existing in &self.direct { | ||
| // Check if two types overlap, by instantiating both types (replacing NamedGenerics | ||
| // with fresh TypeVariables) and then checking if they can unify. | ||
| let instantiate_existing = Self::instantiate_named_generics(&existing.typ, interner); | ||
| let instantiate_typ = Self::instantiate_named_generics(typ, interner); |
There was a problem hiding this comment.
| for existing in &self.direct { | |
| // Check if two types overlap, by instantiating both types (replacing NamedGenerics | |
| // with fresh TypeVariables) and then checking if they can unify. | |
| let instantiate_existing = Self::instantiate_named_generics(&existing.typ, interner); | |
| let instantiate_typ = Self::instantiate_named_generics(typ, interner); | |
| if self.direct.is_empty() { | |
| return None; | |
| } | |
| let instantiate_typ = Self::instantiate_named_generics(typ, interner); | |
| for existing in &self.direct { | |
| // Check if two types overlap, by instantiating both types (replacing NamedGenerics | |
| // with fresh TypeVariables) and then checking if they can unify. | |
| let instantiate_existing = Self::instantiate_named_generics(&existing.typ, interner); |
| if !seen.contains(&id) { | ||
| seen.insert(id); |
There was a problem hiding this comment.
| if !seen.contains(&id) { | |
| seen.insert(id); | |
| if !seen.insert(id) { |
| | Type::Unit | ||
| | Type::Quoted(..) | ||
| | Type::Error | ||
| | Type::TypeVariable(..) |
There was a problem hiding this comment.
I think I have seen a TypeVariable be bound to a NamedGeneric
There was a problem hiding this comment.
Can't explain it, but the example was here: #11385 (comment)
| }) | ||
| .collect(); | ||
|
|
||
| typ.substitute(&substitutions) |
There was a problem hiding this comment.
Just for my understanding: substitute does not actually bind type variables in the typ, it replaces the types that have them (TypeVariable and NamedGeneric) with the substitutions, so NamedGeneric itself is unchanged, right?
If that's right, the docs of substitute could be made clearer.
Description
Problem
Resolves #10540
Summary
Additional Context
User Documentation
Check one:
PR Checklist
cargo fmton default settings.