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

implement defer statement #195

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

implement defer statement #195

wants to merge 2 commits into from

Conversation

mattn
Copy link
Owner

@mattn mattn commented Apr 4, 2018

need tests.

@codecov-io
Copy link

codecov-io commented Apr 4, 2018

Codecov Report

Merging #195 into master will decrease coverage by 1.78%.
The diff coverage is 6.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #195      +/-   ##
==========================================
- Coverage   92.02%   90.24%   -1.79%     
==========================================
  Files           9        9              
  Lines        2270     2317      +47     
==========================================
+ Hits         2089     2091       +2     
- Misses        118      162      +44     
- Partials       63       64       +1
Impacted Files Coverage Δ
vm/env.go 100% <ø> (ø) ⬆️
vm/vm.go 86.14% <ø> (ø) ⬆️
vm/vmStmt.go 89.52% <0%> (-7.48%) ⬇️
vm/vmExprFunction.go 75.53% <33.33%> (-1.48%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fcefaff...e446d16. Read the comment docs.

@mattn
Copy link
Owner Author

mattn commented Apr 4, 2018

Anko's defer statement doesn't work like Go's one since anko doesn't have named return parameters for a function. Anko can not re-write return value. Also current implementation doesn't have recover().

@mattn
Copy link
Owner Author

mattn commented Apr 4, 2018

func foo() {
  var i = 0
  defer println("foo", i)
  i++
  defer println("bar", i)
  i++
  defer println("bar", i)
  i++
  defer println("bar", i)
  i++
  defer println("bar", i)
}

foo()

This works fine.

bar 4
bar 3
bar 2
bar 1
foo 0

@MichaelS11
Copy link
Contributor

Could we put the AnonCallExpr and CallExpr in vmExprFunction.go then be able to use it from case *ast.DeferStmt as well as the current places:

	case *ast.AnonCallExpr:
		return anonCallExpr(e, env)

	case *ast.CallExpr:
		return callExpr(e, env)

Would be really nice if there was a way to merge the two AnonCallExpr & CallExpr so that we do not have to maintain two code areas.

@mattn
Copy link
Owner Author

mattn commented Apr 5, 2018

This implementation is strange since VM run defer even if it is in if/for block. It must be in func block.

var time = import("time")
for {
  defer println("foo")
  time.Sleep(1e9)
}

foo must not be appeared.

@MichaelS11
Copy link
Contributor

MichaelS11 commented Apr 5, 2018

I still wonder if some of this code is not needed. Say they do defer something that is not a function, like a++. If that is passed to say anonCallExpr as an example, that will error and say it is not a function. I think you can just simplify syntax to just DEFER exprs.

@MichaelS11
Copy link
Contributor

Was curious, is it an issue if the code can defer any expression, not just functions?

@xtlsoft
Copy link

xtlsoft commented Jun 3, 2018

I think maybe we need to execute defer statements when an env destroys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants