-
Notifications
You must be signed in to change notification settings - Fork 56
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
[FR]Support Short-circuit for eval bool exp #155
Comments
This may be a useful feature. Feel free to write a pull request. I must unfortunately say though that I do not have enough time at the moment for code reviews for merging it fast. |
Hello, I have a rough understanding of the project and here is the plan I thought of. Lines 323 to 329 in b39bbd7
Maybe we can make the first step lazily: ///Lazy variant of `Node#eval_with_context`
pub fn eval_with_context_lazily<C: Context>(&self, context: &C) -> EvalexprResult<Value> {
self.operator.eval_lazily(&self.children(), context)
} So the actual variable reading from context happens when using it, then the feature might be supported. ...
And => {
expect_operator_argument_amount(children.len(), 2)?;
let left = children[0].eval_with_context_lazily(context)?.as_boolean()?;
Ok(Value::Boolean(left && children[1].eval_with_context_lazily(context)?.as_boolean()?))
},
Or => {
expect_operator_argument_amount(children.len(), 2)?;
let left = children[0].eval_with_context_lazily(context)?.as_boolean()?;
Ok(Value::Boolean(left || children[1].eval_with_context_lazily(context)?.as_boolean()?))
},
Not => {
expect_operator_argument_amount(children.len(), 1)?;
let a = children[0].eval_with_context_lazily(context)?.as_boolean()?;
Ok(Value::Boolean(!a))
},
... What is the best way? Add a lazy variant for |
I would not add new methods for this. Rather have the context type decide if boolean expressions should be evaluated lazily, or not. So add a function to the context trait I think it makes sense to have short circuit evaluation be the default, so the existing context types should just unconditionally return true for this method. |
@ISibboI you can consider using ControlFlow in |
It got an error:
VariableIdentifierNotFound("a")
。But if short-circuit is supported, it should be computeable.The text was updated successfully, but these errors were encountered: