refactored binders to have a private value field #384
Conversation
| let interner = self.db.interner(); | ||
|
|
||
| let (lhs_binders, lhs_bound) = lhs.binders.as_ref().into(); | ||
| let (rhs_binders, rhs_bound) = rhs.binders.as_ref().into(); |
There was a problem hiding this comment.
This is slightly misleading, because as_ref is cloning the binders, whereas it wasn't before.
There was a problem hiding this comment.
binders are cloned later anyway, but I agree, as_ref is kind of a bad name (I just copied it from rustc), should I change it?
There was a problem hiding this comment.
Oh, you mean the names, hmm, ok
There was a problem hiding this comment.
I think as_ref is fine. Just be sure the doc comments are clear. (The binders will almost certainly end up being an interned type instead of a vec anyways, so the clone should be cheap).
jackh726
left a comment
jackh726
left a comment
There was a problem hiding this comment.
Overall, I think this makes things more clean. Couple comments.
| let interner = self.db.interner(); | ||
|
|
||
| let (lhs_binders, lhs_bound) = lhs.binders.as_ref().into(); | ||
| let (rhs_binders, rhs_bound) = rhs.binders.as_ref().into(); |
There was a problem hiding this comment.
I think as_ref is fine. Just be sure the doc comments are clear. (The binders will almost certainly end up being an interned type instead of a vec anyways, so the clone should be cheap).
| pub fn bounds_on_self(&self, interner: &I) -> Vec<QuantifiedWhereClause<I>> { | ||
| let Binders { binders, value } = &self.binders; | ||
|
|
||
| let (binders, assoc_ty_datum) = self.binders.as_ref().into(); |
There was a problem hiding this comment.
This does technically add a clone to binders. But I don't think it matters.
3 participants
Closes #375