-
Notifications
You must be signed in to change notification settings - Fork 276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[refactor] #2001: EvaluatesTo
static type checking
#2582
[refactor] #2001: EvaluatesTo
static type checking
#2582
Conversation
EvaluatesTo
static type checkingEvaluatesTo
static type checking
I'd like to defend my proposal. First, I proposed introducing a trait, not an inherent pub trait ValueIsError<E: Error> {
const VALUE: Self;
fn ok_or(&self, err: E) -> Result<E> {
if *self == VALUE {
err
} else {
Ok(())
}
}
}
impl<E: Error> ValueIsError<(), E> for bool {
const VALUE: Self = false;
}
The point of this isn't to reduce the typing by orders of magnitude, but to allow monadic chaining of such conversions. It might seem stupid to use
|
Yes, it seems like we have contradictory requirements for transaction
We didn't catch that before because the first test was wrong |
@appetrosyan , alright, now the idea of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solution looks good in general, left few suggestions.
Some thoughts: i think when user need to wrpar expression into EvaluatesTo::new_unchecked
it will be reason for double check, however current solution starts to get wordy when it comes nested expressions.
@appetrosyan and I have decided to ignore multisignature test for now. That's will be fixed in #2595 |
@appetrosyan , I've created an issue for |
de3c534
to
2e74385
Compare
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
330b638
to
24a9521
Compare
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
8c7b4b0
to
37fa782
Compare
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job
…hyperledger-iroha#2582) Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
…hyperledger-iroha#2582) Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
…hyperledger-iroha#2582) Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
…hyperledger-iroha#2582) Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
…hyperledger-iroha#2582) Signed-off-by: Daniil Polyakov <arjentix@gmail.com> Signed-off-by: BAStos525 <jungle.vas@yandex.ru>
…hyperledger-iroha#2582) Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
…hyperledger-iroha#2582) Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
…hyperledger-iroha#2582) Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
…hyperledger-iroha#2582) Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
…hyperledger-iroha#2582) Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
…hyperledger-iroha#2582) Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
Description of the Change
EvaluatesTo (#2001)
Into EvaluatesTo<V>
conversion now has stricter bounds which provides static type checkingWhere
,ContextValue
expressions and all queries) so we need a way to explicitly constructEvaluatesTo<V>
omitting the type checking. That's why I've createdEvaluatedTo::<V>::new_unchecked()
impl TryFrom<Value>
it's always safe to constructEvaluatesTo<Value>
. That's why there isEvaluatesTo::<Value>::new_evaluates_to_value()
EvaluatesTo
from uncompatible typeWhile doing that I've found a bug in
Queue
and fixed it. See next section.Tx signature condition checking bug (#2580)
MustUse
type wrapper toprimitives
MustUse
used for a checking functionunused_must_use
warning will be generated ifMustUse
type is unusedIssue
EvaluatesTo
not working #2001Queue
#2580After succesfull review I'll squash my commits into two -- one per resolved issue
Benefits
EvaluatesTo (#2001)
Tx signature condition checking bug (#2580)
MustUse
wrapper typePossible Drawbacks
Harder to construct
EvaluatesTo
from types which require runtime checkUsage Examples or Tests
You can run existing test, because a bunch of them where updated.
Also there are two new tests with
trybuild
:EvaluatesTo
cargo test --package iroha_data_model --test ui -- ui --exact --nocapture
MustUse
cargo test --package iroha_primitives --test ui -- ui --exact --nocapture
Alternate Designs
@appetrosyan suggested to add
ok_or(err)
function toMustUse
which should work something like this:But I have next the thoughts:
std ok_or
does not force you to return()
asOk