go: gccgo: incorrect order of evaluation according to spec

Consider this program:

package main
 
import "fmt"
 
func main() {
        arr := []int{1, 2}
        arr, arr[len(arr)-1] = arr[:len(arr)-1], 3
        fmt.Println(arr)
}

This currently prints [1], and in fact it prints [1] with all versions of Go since Go 1.

According to the spec, when evaluating an assignment statement, all function calls are evaluated in lexical left to right order. Also, all index expressions on the left are evaluated, and then the assignments are done in left to right order.

len is a function call, so all the calls to len should be evaluated before any assignments. This, the second assignment statement should be equivalent to

        arr, arr[2-1] = arr[:2-1], 3

so we have

        arr, arr[1] = arr[:1], 3

So the first assignment in this pair will set arr to [1]. Then the second assignment will set arr[1], but of course there is no such element, so that should panic.

I do not understand why this prints [1] with cmd/compile. With gccgo it panics as I expect with an index out of range error.

CC @griesemer @mdempsky @randall77

About this issue

Original URL
State: closed
Created 7 years ago
Reactions: 3
Comments: 31 (21 by maintainers)

Commits related to this issue

test: add order of evaluation test case that gccgo got wrong Updates #23188 Change-Id: Idc5567546d1c4c592f997a4cebbbf483b85331e0 Reviewed-on: https://go-review.googlesource.com/123115 Reviewed-by: B... — committed to golang/go by ianlancetaylor 6 years ago
compiler: fix evaluation order of LHS index expressions The spec says that when an index expression appears on the left hand side of an assignment, the operands should be evaluated. T... — committed to gcc-mirror/gcc by deleted user 6 years ago
checkers: add evalOrder checker Checks for return statements that may have unwanted dependency on the evaluation order. See: https://github.com/golang/go/issues/27098 https://github.com/golang/go/... — committed to go-critic/go-critic by quasilyte 5 years ago
checkers: add evalOrder checker (#829) Checks for return statements that may have unwanted dependency on the evaluation order. See: https://github.com/golang/go/issues/27098 https://github.co... — committed to go-critic/go-critic by quasilyte 5 years ago

Most upvoted comments

I’m sort of inclined to change the spec to say that if a single statement both refers directly to a variable and changes that variable, either through a direct assignment or a function call, that the result is implementation defined. It shouldn’t be undefined–something plausible should always happens–but we shouldn’t specify the exact ordering of the variable read and write. Then I think we should try to write a vet check for this case.

The argument in favor of this is that no reasonable program should do this. Even if we fully specify the order, anybody reading the program will struggle to understand how it is supposed to work. That doesn’t seem useful. Better to say explicitly that it won’t work reliably, and catch it in vet if we can.

ianlancetaylor on May 28, 2018

I’ve come around to believing that when the spec requires that index operations be evaluated in left-to-right order, that that implies that both operands of the index operation be evaluated. So on that interpretation, the original program in this issue is fully defined, and gccgo gets it wrong.

ianlancetaylor on Jul 10, 2018

@dchenk I don’t know. In a way that is one aspect of what this issue is about.

ianlancetaylor on Jan 31, 2018

@ianlancetaylor I believe the point @mdempsky is making is that arr is also an operand of the index expression, and thus the language requires that arr be evaluated before the assignment. Which would mean that this code should not panic.

That has never been my reading of the spec: I’ve never thought that the spec specified exactly when to read a value out of a variable. We can go that way if we like. I’m concerned that we are making the order of evaluation rules more and more complex without ever fully specifying the order. Do we want to leave room for compiler optimization or not? If we want to leave room, then we should not specify exactly when to read a value from a variable. If we do not want to leave scope, then we should fully specify the order rather than continuing with this ambiguous state. For example, why should we evaluate the variable in an index operation but not in a mathematical operation? I don’t think any sensible program would both set and use the same variable in a single statement, so I don’t think this matters for sensible programs; it’s purely a question of how much compiler optimization the language should permit. I’m OK either way, but I’m not really OK with the fact that we can’t agree on the meaning of a simple statement like the one discussed here.

ianlancetaylor on Jan 30, 2018