code review 6441102: spec: clarify evaluation order

spec: clarify evaluation order

Fixes issue 3852.

[gri, 2012-08-03 21:59:56]

Hello r@golang.org, rsc@golang.org, iant@golang.org, ken@golang.org (cc:
golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[iant, 2012-08-03 22:07:44]

http://codereview.appspot.com/6441102/diff/8001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6441102/diff/8001/doc/go_spec.html#newcode3699
doc/go_spec.html:3699: Unless specified otherwise, when evaluating the <a
href="#Operands">operands</a>
You no longer mention assignments here, but we do want to specify that the
relevant expressions on the LHS of an assignment are evaluated before the ones
on the RHS.

[gri, 2012-08-03 22:14:12]

I think this is still covered in the Assignments section:

"The assignment proceeds in two phases. First, the operands of index
expressions and pointer indirections (including implicit pointer
indirections in selectors) on the left and the expressions on the
right are all evaluated in the usual order. Second, the assignments
are carried out in left-to-right order."

Assignments in this (order of evaluation) section was just one case
now covered by "expression lists".

The reason for the "Unless specified otherwise" is here because in
expression switches we also have expression lists in the case clauses,
but those expressions are evaluated from left to right (as specified
there).

- gri

On Fri, Aug 3, 2012 at 3:07 PM,  <iant@golang.org> wrote:
>
> http://codereview.appspot.com/6441102/diff/8001/doc/go_spec.html
> File doc/go_spec.html (right):
>
> http://codereview.appspot.com/6441102/diff/8001/doc/go_spec.html#newcode3699
> doc/go_spec.html:3699: Unless specified otherwise, when evaluating the
>
> <a href="#Operands">operands</a>
> You no longer mention assignments here, but we do want to specify that
> the relevant expressions on the LHS of an assignment are evaluated
> before the ones on the RHS.
>
> http://codereview.appspot.com/6441102/

[iant, 2012-08-08 15:37:46]

LGTM

http://codereview.appspot.com/6441102/diff/10003/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6441102/diff/10003/doc/go_spec.html#newcode3
doc/go_spec.html:3: "Subtitle": "Version of August 63, 2012",
August 63 is confusing, perhaps write October 2.

[rsc, 2012-08-08 18:32:38]

I know this is nitpicky, but I think the new text is making implicit promises
about things that are not strictly true.

http://codereview.appspot.com/6441102/diff/10003/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6441102/diff/10003/doc/go_spec.html#newcode2099
doc/go_spec.html:2099: The expressions in the element list are
This sentence implies that there is an expressions in the element list are
actually evaluated in some order, but that's not true. If the element list is x
+ y, f(), it is legal to evaluate x before f() and y after, and that doesn't
correspond to any ordering of the expression list.
I would rather leave this out.

http://codereview.appspot.com/6441102/diff/10003/doc/go_spec.html#newcode3699
doc/go_spec.html:3699: Unless specified otherwise, when evaluating the <a
href="#Operands">operands</a>
Where is the possible 'otherwise'? It would be much better to say what the
exceptions are. As written I have to read the rest of the spec to find them. The
only exception to this rule that I can think of is the way select cases get
evaluated, but those are not part of 'assignment, expression, or return
statement' so I think that's fine. You might be able to just not say 'Unless
specified otherwise'.

This sentence also, like the one above, shifts the focus from subexpressions to
lists of expressions, which I think is misleading. The old text did not mention
lists at all, and I think that is preferable.

I think the only case that was missing from the old text was return statements,
so maybe just replace "assignment or expression" with "assignment, expression,
or return statement" and remove this explicit enumeration of lists.

http://codereview.appspot.com/6441102/diff/10003/doc/go_spec.html#newcode4503
doc/go_spec.html:4503: The expressions are <a
href="#Order_of_evaluation">evaluated in the usual order</a>.
Again this makes it sound like there is an ordering among the expressions, when
in fact they're all kind of evaluated simultaneously and the pieces fall
wherever.

I'd rather just mention 'or return statement' above and leave this reference
out.

Then the only part of the spec that makes claims about order of evaluation is
the order of evaluation section.

[iant, 2012-08-08 18:59:03]

On 2012/08/08 18:32:38, rsc wrote:

> Where is the possible 'otherwise'?

The && and || operators are also a case where expressions are not evaluated in
strict left-to-right lexical order.

I'm fine with simplicity and trying to keep the "order of evaluation" text in a
single section.

[gri, 2012-08-08 20:22:41]

PTAL.

http://codereview.appspot.com/6441102/diff/10003/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6441102/diff/10003/doc/go_spec.html#newcode3
doc/go_spec.html:3: "Subtitle": "Version of August 63, 2012",
On 2012/08/08 15:37:46, iant wrote:
> August 63 is confusing, perhaps write October 2.

Print 3 on top of 6 and you get 8, the correct date. There's a missing
backspace.

http://codereview.appspot.com/6441102/diff/10003/doc/go_spec.html#newcode2099
doc/go_spec.html:2099: The expressions in the element list are
On 2012/08/08 18:32:38, rsc wrote:
> This sentence implies that there is an expressions in the element list are
> actually evaluated in some order, but that's not true. If the element list is
x
> + y, f(), it is legal to evaluate x before f() and y after, and that doesn't
> correspond to any ordering of the expression list.
> I would rather leave this out.

Done.

http://codereview.appspot.com/6441102/diff/10003/doc/go_spec.html#newcode3699
doc/go_spec.html:3699: Unless specified otherwise, when evaluating the <a
href="#Operands">operands</a>
On 2012/08/08 18:32:38, rsc wrote:
> Where is the possible 'otherwise'? It would be much better to say what the
> exceptions are. As written I have to read the rest of the spec to find them.
The
> only exception to this rule that I can think of is the way select cases get
> evaluated, but those are not part of 'assignment, expression, or return
> statement' so I think that's fine. You might be able to just not say 'Unless
> specified otherwise'.

Otherwise is true for select cases but also for logical operations && and ||.

http://codereview.appspot.com/6441102/diff/10003/doc/go_spec.html#newcode4503
doc/go_spec.html:4503: The expressions are <a
href="#Order_of_evaluation">evaluated in the usual order</a>.
On 2012/08/08 18:32:38, rsc wrote:
> Again this makes it sound like there is an ordering among the expressions,
when
> in fact they're all kind of evaluated simultaneously and the pieces fall
> wherever.
> 
> I'd rather just mention 'or return statement' above and leave this reference
> out.
> 
> Then the only part of the spec that makes claims about order of evaluation is
> the order of evaluation section.

Done.

[r, 2012-08-08 21:51:28]

LGTM

http://codereview.appspot.com/6441102/diff/12003/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6441102/diff/12003/doc/go_spec.html#newcode3723
doc/go_spec.html:3723: x := []int{a, f()}  // x may be [1, 3] or [2, 3] because
the evaluation order between a and f() is not specified
you can s/ because the/:/ to shorten the line.

[gri, 2012-08-08 22:04:01]

PTAL.

Any others?

http://codereview.appspot.com/6441102/diff/12003/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6441102/diff/12003/doc/go_spec.html#newcode3723
doc/go_spec.html:3723: x := []int{a, f()}  // x may be [1, 3] or [2, 3] because
the evaluation order between a and f() is not specified
On 2012/08/08 21:51:28, r wrote:
> you can s/ because the/:/ to shorten the line.

Done.

[iant, 2012-08-08 22:13:26]

LGTM

[gri, 2012-08-09 18:50:21]

*** Submitted as http://code.google.com/p/go/source/detail?r=6beada1bf6c2 ***

spec: clarify evaluation order

Fixes issue 3852.

R=r, rsc, iant, ken
CC=golang-dev
http://codereview.appspot.com/6441102

[rsc, 2012-08-09 20:11:07]

LGTM, thanks