Make find_similar_impl_candidates a little fuzzier.
#92223
Conversation
rustbot
added
the
T-compiler
rust-highfive
added
the
S-waiting-on-review
There was a problem hiding this comment.
it might be meaningful to also sort by whether the non-self parameters of the impls also match the searched ones.
would be nice to try this here if you're interested, otherwise we can also merge this as is.
i might not be available over the holidays, so you may have to wait a week until i can come back to you
| @@ -56,7 +57,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { | |||
| trait_ref.substs.types().skip(1), | |||
| impl_trait_ref.substs.types().skip(1), | |||
| ) | |||
| .all(|(u, v)| self.fuzzy_match_tys(u, v)) | |||
| .all(|(u, v)| self.fuzzy_match_tys(u, v, StripReferences::No)) | |||
There was a problem hiding this comment.
are there unwanted changes when using StripReferences::Yes here?
| // Pass `StripReferences::Yes` because although we do want to | ||
| // be fuzzier than `simplify_type`, we don't want to be | ||
| // *too* fuzzy. |
There was a problem hiding this comment.
that reasoning seems unclear to me 🤔 how does passing StripReferences::Yes prevent ourselves from being "too fuzzy"?
| // Check for match between simplified types. | ||
| let imp_simp = fast_reject::simplify_type( | ||
| self.tcx, | ||
| imp.self_ty(), | ||
| SimplifyParams::Yes, | ||
| StripReferences::Yes, | ||
| ); | ||
| if let Some(imp_simp) = imp_simp { | ||
| if simp == imp_simp { | ||
| return Some(ImplCandidate { | ||
| trait_ref: imp, | ||
| similarity: CandidateSimilarity::Simplified, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
i don't believe that this special case has a meaningful advantage over only using exact matches and fuzzy matches.
I would prefer removing this and removing the StripReferences function parameter from simplify_type. This should be the only place where it is currently used.
lcnr
added
S-waiting-on-author
JohnCSimon
added
S-waiting-on-author
|
Ping from triage: |
|
Sorry, been a lot of other stuff on my mind recently. I don't have immediate plans to continue on this, unless someone really wants this to get in. |
|
@lcnr I don't mind at all, thank you for carrying the torch for me! |
make `find_similar_impl_candidates` even fuzzier continues the good work of `@BGR360` in rust-lang#92223. I might have overshot a bit and we're now slightly too fuzzy 😅 with this we can now also simplify `simplify_type`, which is nice :3
Resolves #92113
This is the initial draft, I'm curious for your thoughts. Still need to write ui tests for the arrays/slices case.
Let me know if anything is unclear; I stayed up far too late and am far too tired to document my decisions in more detail at this very moment 😁
r? @lcnr