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

cmd/compile: evaluate map initializers incrementally #26552

Open
randall77 opened this Issue Jul 23, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@randall77
Contributor

randall77 commented Jul 23, 2018

func g() int
func f() map[int]int {
	return map[int]int{
		1: g(),
		2: g(),
		3: g(),
	}
}

The generated assembly does essentially:

    m = make(map[int]int)
    v1 = g()
    v2 = g()
    v3 = g()
    m[1] = v1
    m[2] = v2
    m[3] = v3

It would be better to evaluate the function g just before we need its result. So instead, do:

    m = make(map[int]int)
    v1 = g()
    m[1] = v1
    v2 = g()
    m[2] = v2
    v3 = g()
    m[3] = v3

It should be safe to do so, as when we're constructing m no one but the current stack frame has a reference to it, so g can't see the difference.

See #26546 for an instance where this matters.

@randall77 randall77 added this to the Go1.12 milestone Jul 23, 2018

@josharian

This comment has been minimized.

Contributor

josharian commented Jul 23, 2018

Are there order-of-evaluation guarantees for map literals? If so, the difference might be observable if one of the calls to g() panics.

@cherrymui

This comment has been minimized.

Contributor

cherrymui commented Jul 23, 2018

If g() panics, the map literal should be inaccessible. So I think it should be fine.

@randall77

This comment has been minimized.

Contributor

randall77 commented Jul 23, 2018

We do need to respect order of evaluation, so there are tricky cases like:

func f() map[int]int {
	return map[int]int{
		k1(): v1(),
		k2(): v2(),
		k3(): v3(),
	}
}

Spec says the eval order must be k1,v1,k2,v2,k3,v3. But that should work fine with incremental building.

@randall77

This comment has been minimized.

Contributor

randall77 commented Dec 12, 2018

Punting to 1.13, too late for anything major in 1.12.

@randall77 randall77 modified the milestones: Go1.12, Go1.13 Dec 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment