Unexpected evaluation order in a return statement with multiple operands

[Tom Payne]

The Go language specification on order of evaluation states:

  "when evaluating the operands of an expression, assignment, or return statement, all function calls, method calls, and communication operations are evaluated in lexical left-to-right order."

Consider the following program (also in the Go Playground):

  package main

  import (

      "fmt"

  )

  func modify(i *int) error {

      *i = 1

      return nil

  }

  func f() (int, error) {

      i := 0

      return i, modify(&i)

  }

  func main() {

      i, _ := f()

      fmt.Printf("i == %d\n", i)

  }

I would expect return value from f() to be 0, nil, as, evaluated left to right the operands have values 0 and nil, but instead the return value is 1, nil.

What is the explanation for what's actually happening here? Can I rely on this behavior or might it change in future versions of Go?

Cheers,

Tom

[Paul Jolly]

> "when evaluating the operands of an expression, assignment, or return statement, all function calls, method calls, and communication operations are evaluated in lexical left-to-right order."

My understanding goes as follows: the operands of the return statement
are i and modify(&i). The comma after "return statement" in the above
sentence is then important: because the only "function calls, method
calls, and communication operations" in that list of operands are (is)
modify(&i).

Hence when i (as in the first operand) is evaluated is not specified.
And therefore it's dangerous to rely on it being one value or another
(which it could be given the example you provide).

There was even some discussion at the London Gophers November 2019
meetup (where the conundrum was very related
https://docs.google.com/presentation/d/e/2PACX-1vQ07uP_ldnSYzpaNb5AlZZ-_tL2mZuoNfQgxvsTKSM4aglYR-nuvyrZ8nK__r3YQTo1vqH-Hmax3aXs/pub?slide=id.g6239d05d0e_0_1)
about whether it would be possible to statically flag potential errors
like this.

[Robert Engels]

I think the way to look at it is “what would be the behavior of if you were inline creating and initializing a struct containing both values” - clearly this behavior is not what you would want or expect.

> On Jan 15, 2020, at 1:25 PM, Paul Jolly <pa...@myitcv.io> wrote:
>
> 

> --
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CACoUkn7KA0Z-TDypdvM%3Du%3DyVPHuHFmtD%3DiTV2c98Vm%3Dqn4NcPw%40mail.gmail.com.

[Axel Wagner]

And, just as an addendum: IMO, if the result depends on the order of operations in a statement, prefer to rewrite it. Like, even if you are satisfied with the behavior you are seeing *and* you can rely on it - it should be clear by now that anyone looking at it will have to expand new effort understanding it (and, you know, there might still be bugs in the future). Rewriting the function call into its own statement gets rid of the ambiguity and makes clear what's happening either way.

[Axel Wagner]

I don't think "clearly this behavior is not what you would want" is a valid generalization.

  type MyStruct{

      // …

  }

  func (x *MyStruct) initialize() error {
      // does some common verification and populates some internal data structures
      return nil
  }

  func New() (*MyStruct, error) {

      x := &MyStruct{

           // default fields

      }

      return x, x.initialize()

  }

  func NewWithFoo(f Foo) (*MyStruct, error) {

       x := &MyStruct{

            Foo: f,

       }

       return x, x.intialize()

  }

You can say that you shouldn't do something like this, but it's IMO reasonable code to write and it expects different behavior.

However, as per my previous e-mail, I would still prefer to rewrite this, of course :)

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/9EC93CAE-B11C-45E3-9D01-AC4AE5769BEE%40ix.netcom.com.

[Robert Engels]

I disagree here. Knowing the order of operations/evaluation is part of knowing the language. If that’s the case it should be undefined (which forces a rewrite) not chose a behavior that seems contrary to the documentation and common practice in this area. 

On Jan 15, 2020, at 1:39 PM, Axel Wagner <axel.wa...@googlemail.com> wrote:



[Tom Payne]

Thanks all for the insight and explanation. Could I suggest tweaking the Go language specification to emphasize the separation, so it reads:

   "when evaluating the operands of an expression, assignment, or return statement, then all function calls, method calls, and communication operations are evaluated in lexical left-to-right order. Any operands that are variables take the value of the variable after all function calls, methods calls, and communication operations have been evaluated."

The presence or absence of function calls does make a difference here. In this example the use of the identity() function changes the result of f():

  func identity(i int) int {
      return i

  }

  func modify(i *int) error {
      *i = 1
      return nil
  }

  func f() (int, error) {
      i := 0

      return identity(i), modify(&i) // returns 0, nil

  }

You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/Q7KVGTFt3nU/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/1388A9A2-F321-459A-893E-6D90D42E7255%40ix.netcom.com.

[Robert Engels]

That is why this should be tightened down imo. It’s obscure cruft that needs to be accounted for when refactoring. 

On Jan 15, 2020, at 2:23 PM, Tom Payne <t...@tompayne.org> wrote:



[Ian Lance Taylor]

On Wed, Jan 15, 2020 at 12:24 PM Tom Payne <t...@tompayne.org> wrote:
>
> Thanks all for the insight and explanation. Could I suggest tweaking the Go language specification to emphasize the separation, so it reads:
>
> "when evaluating the operands of an expression, assignment, or return statement, then all function calls, method calls, and communication operations are evaluated in lexical left-to-right order. Any operands that are variables take the value of the variable after all function calls, methods calls, and communication operations have been evaluated."

That suggested addition is not correct. The language deliberately
does not specify when operands that are variables are evaluated. They
may be evaluated before function calls, they may be evaluated after.
Different Go compilers make different decisions here.

If we decided to specify it, I'm sure we would specify that the
variables are evaluated in left to right order.

Personally I think one good approach would be a vet check that
complains about statements that both read and write a variable in an
unspecified order. But it's fairly hard to catch all possible cases
of this.

Ian

[Tom Payne]

Thanks again - I'm learning a lot here!

For info, the background here is that I've been writing code like:

  type T struct {

      SomeField int

  }

  func NewTFromJSON(data []byte) (*T, error) {

      var t T

      return &t, json.Unmarshal(data, &t)

  }

which wraps up error handling into a single line. This works when T is a non-simple type as &t does not change, even if the value of t does.

Writing this with T being a simple type, like int, results in the case described in the original post:

  // NewIntFromJSON relies on undefined behavior, don't use it!

  func NewIntFromJSON(data []byte) (int, error) {

      var i int

      return i, json.Unmarshal(data, &i)

  }

So, in these cases I will do as Axel suggests and avoid the implicit dependence on evaluation order.

A go vet check for this would be wonderful, even if it's not perfect. As long as there aren't any false positives (i.e. warning when the code does not in fact rely on unspecified behavior), it matters less if there are false negatives (i.e. not warning when the code actually relies on unspecified behavior). Maybe a simple test could catch the most common cases.

[Jake Montgomery]

I could not agree more with Axel's assessment, and wanted to emphisize it for any Go-noobs reading this. An important part of the go philosophy is to write code that is as clear and simple as possible. It is not about compact code, or 'clever' code. Code should be easy to understand, even when skimming. A return statement that returns a value, and modifies it in the same statement, would not pass code review for me.

Just because you can, does not mean you should.

To unsubscribe from this group and stop receiving emails from it, send an email to golan...@googlegroups.com.