Guarantees about evaluation order but not slice indexing

[Dmitriy Cherchenko]

It looks like the language spec is a little imprecise in the Order of evaluation section about if any function evaluating an index of a slice will always be invoked before anything is done to the slice at that index. That is, if you have something like:

    mySlice[someFunc()] = 45

is someFunc() guaranteed to be called before the value at the int it returns is set to 45?

I created a sample test and ran it on 32 different kinds of machines (using Travis CI), including 8 different versions of Go, two different architectures, and two operating systems.

Here is the code: https://github.com/dchenk/go-eval-order

And here are the test results: https://travis-ci.org/dchenk/go-eval-order/builds/335042572

So what are the guarantees, if there are any, about the evaluation order of the inner function verses the slice indexing operation? I want to make sure invalid indexing isn't possible in the kind of code on line 10 in the main.go file in the repo linked to above.

Thanks!

[Axel Wagner]

On Tue, Jan 30, 2018 at 9:10 AM, Dmitriy Cherchenko <dcher...@gmail.com> wrote:

It looks like the language spec is a little imprecise in the Order of evaluation section about if any function evaluating an index of a slice will always be invoked before anything is done to the slice at that index. That is, if you have something like:

    mySlice[someFunc()] = 45

is someFunc() guaranteed to be called before the value at the int it returns is set to 45?

I don't understand this question. How could it possibly not be? It has to be evaluated to know what value it returns?

I created a sample test and ran it on 32 different kinds of machines (using Travis CI), including 8 different versions of Go, two different architectures, and two operating systems.

Here is the code: https://github.com/dchenk/go-eval-order

And here are the test results: https://travis-ci.org/dchenk/go-eval-order/builds/335042572

FWIW, the failing test cases seem to be flakes - i.e. they are timed out builds, not actual test failures?

So what are the guarantees, if there are any, about the evaluation order of the inner function verses the slice indexing operation? I want to make sure invalid indexing isn't possible in the kind of code on line 10 in the main.go file in the repo linked to above.

From my reading of the spec, I'd argue that the code is potentially incorrect, but I'm having trouble parsing your question above into what I consider a potential failure scenario.

For the line

b.data[b.grow()] = bt

the spec explicitly makes no guarantees about the evaluation order of b.data, b.grow, b.grow() and bt, as it only orders function/method/communication calls among each other. These are roughly the evaluations that happen:

1. n := b.grow()

2. s := b.data

3. s[n] = bt

However, this would also be valid (from my reading of the spec):

1. s := b.data

2. n := b.grow()

3. s[n] = bt

In that case, s might have a stale copy of the slice, from before the append. To fix that, you should split that into two lines:

func (b *Buf) WriteByte(bt byte) {

    n := b.grow()

    b.data[n] = bt

}

Hope that helps.

[Dmitriy Cherchenko]

The second possibility you described is exactly what I’m worried about. But I have several places where I’m doing something like b.data[b.grow()] = bt and wouldn’t want to break that up into two lines everywhere. I was wondering if you could find anything else in the spec to guarantee that the index (b.grow()) is evaluated first, before the slice is identified.

[Ian Lance Taylor]

On Tue, Jan 30, 2018 at 10:20 AM, Dmitriy Cherchenko
<dcher...@gmail.com> wrote:
>
> The second possibility you described is exactly what I’m worried about. But I have several places where I’m doing something like b.data[b.grow()] = bt and wouldn’t want to break that up into two lines everywhere. I was wondering if you could find anything else in the spec to guarantee that the index (b.grow()) is evaluated first, before the slice is identified.

That is an interesting example. See the current discussion on
https://golang.org/issue/23188.

Ian

[Axel Wagner]

On Tue, Jan 30, 2018 at 7:20 PM, Dmitriy Cherchenko <dcher...@gmail.com> wrote:

The second possibility you described is exactly what I’m worried about. But I have several places where I’m doing something like b.data[b.grow()] = bt and wouldn’t want to break that up into two lines everywhere. I was wondering if you could find anything else in the spec to guarantee that the index (b.grow()) is evaluated first, before the slice is identified.

I don't think right now you can (though, TBH, I'm not great at rules-lawyering prose). I'm not saying you shouldn't be (i.e. I'm not saying that specifying this wouldn't be a good thing to do), just that with the spec as it is, this is not specified and you have to decide whether to trust that implementations behave as expected (and leave it in one line) or want to be safe and split it up in two.

I think the code you posted is completely reasonable code and it is completely reasonable to expect it to behave as you intend - which is why I think the spec should somehow be amended to make it work that way (otherwise it would make it far too easy for a bug to sneak through code review). But until then, the spec is as it is.

You might want to add this example to the bug Ian linked (or one of the related ones linked from there).