feat: Allow predicates to own object and evaluate against borrowed types #134
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
epage
reviewed
Dec 28, 2022
src/ord.rs
Outdated
| } | ||
| } | ||
|
|
||
| fn find_case<'b>(&'b self, expected: bool, variable: &T) -> Option<reflection::Case<'b>> { | ||
| fn find_case<'a>(&'a self, expected: bool, variable: &T) -> Option<reflection::Case<'a>> { |
There was a problem hiding this comment.
If we're splitting things out into separate commits, please split this one oput
It is very useful to be able to dynamically construct an object and have that object owned by the predicate, yet evaluate against an unowned type related to the owned one. An obvious example is a String being owned by the predicate but being compared against &strs. Therefore, implement Predicate for Eq/OrdPredicate that store an object that implements Borrow for the predicate type, replacing existing impls of Predicate<T> for Eq/OrdPredicate<T> and Eq/OrdPredicate<&T>. This is backwards compatible as there are blanket implementations of Borrow<T> for T and Borrow<T> for &T. Note that Borrow imposes more requirements than are actually required and AsRef would be sufficient. However, AsRef doesn't have a blanket implementation for T and thus the existing impl of Predicate<T> for EqPredicate<T> is still required, but results in a conflict since T may also implement AsRef<T>. Requiring Borrow instead of AsRef is sufficient for common use cases though. This addresses assert-rs#20 more completely.
…ypes It is very useful to be able to dynamically construct an object and have that object owned by the predicate, yet evaluate against an unowned type related to the owned one. An obvious example is a String being owned by the predicate but being compared against &strs. Therefore, implement Predicate for In/OrdIn/HashInPredicate that store an object that implements Borrow for the predicate type, replacing existing impls of Predicate<T> for In/OrdIn/HashInPredicate<T> and In/OrdIn/HashInPredicate<&T>. This is backwards compatible as there are blanket implementations of Borrow<T> for T and Borrow<T> for &T. Note that Borrow imposes more requirements than are actually required and AsRef would be sufficient. However, AsRef doesn't have a blanket implementation for T and thus the existing impl of Predicate<T> for InPredicate<T> is still required, but results in a conflict since T may also implement AsRef<T>. Requiring Borrow instead of AsRef is sufficient for common use cases though. This addresses assert-rs#20 more completely.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is very useful to be able to dynamically construct an object and have that object owned by the predicate, yet evaluate against an unowned type related to the owned one. An obvious example is a String being owned by the predicate but being compared against &strs.
Therefore, implement Predicate for Eq/OrdPredicate that store an object that implements Borrow for the predicate type, replacing existing impls of Predicate for Eq/OrdPredicate and Eq/OrdPredicate<&T>. This is backwards compatible as there are blanket implementations of Borrow for T and Borrow for &T.
Note that Borrow imposes more requirements than are actually required and AsRef would be sufficient. However, AsRef doesn't have a blanket implementation for T and thus the existing impl of Predicate for EqPredicate is still required, but results in a conflict since T may also implement AsRef. Requiring Borrow instead of AsRef is sufficient for common use cases though.
This addresses #20 more completely.