Go: gccgo: "range x" evaluates x even when it shouldn't

[mdempsky]

The Go spec says (though see also #22258):

The range expression is evaluated once before beginning the loop, with one exception: if the range expression is an array or a pointer to an array and at most one iteration variable is present, only the range expression's length is evaluated; if that length is constant, by definition the range expression itself will not be evaluated.

In the program below *p has a constant length (and even len(*p) is constant) and there's at most one iteration variable present, so *p should not be evaluated:

package main

func main() {
        var p *[10]int
        for range *p {
        }
}

When compiled with cmd/compile, this program runs and exits normally. (Seemingly according to spec.)

When compiled with gccgo, this program panics because of a nil pointer dereference. (Seemingly contrary to the spec.)

/cc @ianlancetaylor @griesemer

[mdempsky]

for _, _ = range *p {} should not panic either, because

If the last iteration variable is the blank identifier, the range clause is equivalent to the same clause without that identifier.

[bcmills]

Just curious: why did we add that caveat to the spec in the first place? It's annoyingly inconsistent with the general “range expression is evaluated once” rule...

[mdempsky]

@bcmills I think so that

for i := range x { ... }

is equivalent to

for i, n := 0, len(x); i < n; i++ { ... }

[bcmills]

I was going to say that they're already not equivalent, but now I don't even know.
https://play.golang.org/p/P6c1ho-y1M

The current phrasing of the spec suggests that this program should not evaluate the call to panicArray, but it is evaluated nonetheless:

func panicArray() [3]int {
    panic("nope")
}

func main() {
    for i := range panicArray() {
        fmt.Println(i)
    }
} 

[griesemer]

len(panicArray()) is not a constant per the spec.

[bcmills]

len(panicArray()) is not a constant per the spec.

That's my point: according to the current phrasing, it seems like for i := range panicArray() should evaluate only the length of the panicArray() expression, but for i, n := 0, len(panicArray()); … should evaluate the expression itself.

“if the range expression is an array or a pointer to an array and at most one iteration variable is present, only the range expression's length is evaluated” is not conditioned upon whether the expression is a constant.

I suppose that depends on fairly subtle details about what it means to “evaluate” the length of an array of constant size returned by a function call.

(Contrast len vs. unsafe.Sizeof: https://play.golang.org/p/1rjtYb3Gdw)

[griesemer]

That's why the new phrasing, suggested via #22258 ( https://go-review.googlesource.com/c/go/+/71431/3/doc/go_spec.html ) makes this clearer.

The range expression x is evaluated (with a single iteration variable) only if len(x) is not a constant. And the spec is clear abut when len(x) is a constant: https://golang.org/ref/spec#Length_and_capacity . It's definitively not a constant if there's a function call in x, which is what we have here.

Note that unsafe.Sizeof are always compile-time constant expressions per the spec.

[mdempsky]

Oh, I just noticed there's even this example in the spec, which demonstrates the same bug:

var testdata *struct {
    a *[7]int
}
for i, _ := range testdata.a {
    // testdata.a is never evaluated; len(testdata.a) is constant
    // i ranges from 0 to 6
    f(i)
}

[gopherbot]

Change https://golang.org/cl/91555 mentions this issue: compiler: don't incorrectly evaluate range variable

[gopherbot]

Change https://golang.org/cl/91715 mentions this issue: compiler: in range, evaluate array if it has receives or calls