Go: cmd/compile: generate closure for go/defer builtin calls

[unixpickle]

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

Go 1.8.
Also happens on play.golang.org.

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"

What did you do?

Evaluate this code:

package main

import "fmt"

func main() {
    fmt.Println("should be 0:", len(foobar()))
}

func foobar() map[int]bool {
    m := map[int]bool{}
    for i := 0; i < 3; i++ {
        m[i] = true
        defer delete(m, i)
    }
    return m
}

What did you expect to see?

should be 0: 0

What did you see instead?

should be 0: 2

The defer statement is supposed to freeze its arguments the instant it is evaluated, but it does not when used with delete. Compare to this program, which works correctly:

package main

import "fmt"

func main() {
    fmt.Println("should be 0:", len(foobar()))
}

func foobar() map[int]bool {
    m := map[int]bool{}
    for i := 0; i < 3; i++ {
        m[i] = true
        defer myDelete(m, i)
    }
    return m
}

func myDelete(m map[int]bool, k int) {
    delete(m, k)
}

[mattn]

This is my interesting. Why you use defer delete?

[unixpickle]

@mattn I have a map that I occasionally add temporary values to (for intermediate computations). I was trying to use defer+delete to remove those temporary entries automatically.

[bradfitz]

Good news and bad news:

Bad news is this has been broken forever. (I tested Go 1.5, Go 1.6, Go 1.7, and Go 1.8) Because it's not a regression and has been broken forever, it doesn't qualify for a Go 1.8.1 cherry-pick.

Good news is that it's fixed at tip (what will be Go 1.9).

I haven't bisected to see where it was fixed and whether it was intentional. If it was fixed by accident, it likely still lacks a test.

/cc @griesemer @randall77 @mdempsky @josharian

[josharian]

Probably fixed by the order.go changes for mapdelete_fast*. Yes, needs a test. Should double-check go delete(x), and defer/delete of the other built-in functions.

[cespare]

A bisect points at 5d6b7fcaa1444f6c17d519c9ce7bc0771bfd96ec (CL 38172)

runtime: add mapdelete_fast*

[josharian]

order.go, walk.go, and ssa.go are scattered with special handling of go/defer of built-in functions. I wonder whether instead we should just do a very early rewrite of

defer builtin(a, b, c)

to

defer func(a typa, b typb, c typc) {
  builtin(a, b, c)
}(a, b, c)

That would probably also simplify instrumentation of such cases, which are also the topic of scattered special handling.

[randall77]

@josharian I was thinking the same thing.

[josharian]

I think we should add a test for 1.9 and and leave the closurization of deferred built-ins for 1.10. (That will also enable us to simplify other things, like not lowering OPANIC in walk.go.)

I'll send a test CL.

[gopherbot]

CL https://golang.org/cl/43497 mentions this issue.

[josharian]

Note to self: walkprintfunc does this now for defer println(...). It also has a minor naming bug: lookupN("print·%d", walkprintfunc_prgen) generates names like "".print·%d1·f. :P

[cuonglm]

This is fixed at tip, we can close this now cc @mdempsky