chore: simplify associated constants lookup#9133
Conversation
| let named = vecmap(&self.associated_types, |generic| { | ||
| let name = Ident::new(generic.name.to_string(), location); | ||
| NamedType { name, typ: generic.clone().as_named_generic() } | ||
| NamedType { name, typ: Type::TypeVariable(generic.type_var.clone()) } |
There was a problem hiding this comment.
@jfecher When I initially started this PR some tests were failing. Comparing with master I noticed that when we elaborate AsTraitPath the TraitGenerics we produce there have type variables for named arguments. Here we were returning NamedGeneric and that didn't work (they don't unify the same way as type variables). Given that the AsTraitPath was working well, I decided to change this code to also return TypeVariable. That fixed the issue but I'm not 100% sure this is fine.
![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 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 92c2c61 | Previous: d31ebb9 | Ratio |
|---|---|---|---|
semaphore-depth-10 |
0.028 s |
0.022 s |
1.27 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
1 participant
Description
Problem
No issue, just something I noticed.
Summary
After #9041 there were a couple of code paths that were no longer needed. That is, #9041 was the correct way to implement associated constants, the other code wasn't a workaround but it didn't account for all cases.
Additional Context
Documentation
Check one:
PR Checklist
cargo fmton default settings.