Skip to content
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

Lazy evaluation working, closes #221 #227

Merged
merged 1 commit into from May 26, 2019

Conversation

Projects
None yet
2 participants
@mingwho
Copy link
Contributor

commented May 16, 2019

$ x = false
$ x && x.y
false

@mingwho mingwho force-pushed the mingwho:lazy-eval branch from da877d6 to 9316ea5 May 16, 2019

if isError(right) {
// If operator is "&&" or "||", we don't return right here to allow lazy evaluation
// right will be returned later in evalInfixExpression when lazy evlation doesn't apply
if isError(right) && !(node.Operator == "&&" || node.Operator == "||") {

This comment has been minimized.

Copy link
@odino

odino May 18, 2019

Collaborator

This code will still make it so that the right side is evaluated. Imagine we have something like:

true || 1..1000000000

This expression will take ages to evaluate even though it could theoretically return immediately.

What I'd suggest is to change evalInfixExpression to accept nodes rather than their evaluated value, and let it handle lazy evaluation on its own. Something like:

evalInfixExpression(operator, left, right) {
  leftVal = eval(left)

  if !isTruthy(leftVal) {
    if operator = && {
    return eval(right)
   }
    return false
  }

  if isTruthy(leftVal) && operator = || {
    return leftVal
  }

  rightVal = eval(right)

  switch:
    // all other cases
}

This comment has been minimized.

Copy link
@mingwho

mingwho May 19, 2019

Author Contributor

I was thinking about this solution! This is more efficient (lazy) 👍

@mingwho mingwho force-pushed the mingwho:lazy-eval branch 2 times, most recently from ea62458 to 1e3d1c1 May 21, 2019

@mingwho mingwho force-pushed the mingwho:lazy-eval branch from 1e3d1c1 to bc7a96c May 21, 2019

@odino odino merged commit cd22152 into abs-lang:1.4.x May 26, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
coverage/coveralls Coverage remained the same at 70.785%
Details
@odino

This comment has been minimized.

Copy link
Collaborator

commented May 26, 2019

Thanks @mingwho! Just tested and works like a charm. I will be adding a benchmark as well.

odino added a commit that referenced this pull request May 26, 2019

odino added a commit that referenced this pull request May 26, 2019

odino added a commit that referenced this pull request May 27, 2019

odino added a commit that referenced this pull request Jun 6, 2019

odino added a commit that referenced this pull request Jun 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.