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

cmd/compile: comma ok assignment order is right-to-left for some statements #13433

Closed
trisiak opened this issue Nov 30, 2015 · 3 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@trisiak
Copy link

trisiak commented Nov 30, 2015

When the same variable is used twice in a "comma ok" statement, the assignment is sometimes carried out right-to-left:

The specification is consistent and states that:

(...) the assignments are carried out in left-to-right order.

From reading of how OAS2MAP is handled this is clearly an explicit action since the compiler does a rewrite of the statement (https://github.com/golang/go/blob/go1.5.1/src/cmd/compile/internal/gc/walk.go#L808-L881). I could not find the reason for why OAS2DOTTYPE doesn't follow the specification.

This is the case for at least go1.5.1.

@ianlancetaylor ianlancetaylor added this to the Go1.7 milestone Dec 1, 2015
@rsc rsc modified the milestones: Go1.8, Go1.7 May 17, 2016
@quentinmit quentinmit added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 11, 2016
@rsc
Copy link
Contributor

rsc commented Oct 21, 2016

Single-program test case: https://play.golang.org/p/ZJgUHH-Qjn

package main

var c = make(chan bool, 10)
func init() { c <- false; c <- false }
var i interface{} = false
func f() (bool, bool) { return false, true }
var m = map[int]bool{0: false}

func main() {
    c <- false

    var x, y bool

    x, y = m[0]
    println("x, y = m[0]:", x, y)
    x, x = m[0]
    println("x, x = m[0]:", x, x, "\n")

    x, y = <-c
    println("x, y = <-c:", x, y)
    x, x = <-c
    println("x, x = <-c:", x, x, "\n")

    x, y = f()
    println("x, y = f():", x, y)
    x, x = f()
    println("x, x = f():", x, x, "\n")

    x, y = i.(bool)
    println("x, y = i.(bool):", x, y)
    x, x = i.(bool)
    println("x, x = i.(bool):", x, x)
}

prints:

x, y = m[0]: false true
x, x = m[0]: false false 

x, y = <-c: false true
x, x = <-c: true true 

x, y = f(): false true
x, x = f(): true true 

x, y = i.(bool): false true
x, x = i.(bool): false false

All the x, x should be true, true, not false, false, because the right-side x should be assigned second.

Great bug. These don't follow the spec because it never occurred to me that they'd be assigning to the same variable, and it was often cheaper to set the "ok" before the "real result".

This might be fixable in order.go rather than in each case.

@rsc rsc modified the milestones: Go1.9, Go1.8 Oct 21, 2016
@rsc rsc added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Oct 21, 2016
@gopherbot
Copy link

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

@JayNakrani
Copy link
Contributor

golang.org/cl/34713 does the reordering by introducing temporaries in order.go. Dump of main() with the change is here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants