Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1175)

Issue 99020043: code review 99020043: spec: clarify section on package initialization (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 10 months ago by gri
Modified:
9 years, 10 months ago
Reviewers:
r, rsc, adonovan, iant
CC:
r, rsc, iant, ken2, adonovan, golang-codereviews
Visibility:
Public.

Description

spec: clarify section on package initialization - split description of package initialization and program execution - better grouping of concerns in section on package initialization - more explicit definition of what constitues a dependency - removed language about constant dependencies - they are computed at compile-time and not initialized at run-time - clarified that independent variables are initialized in declaration order (rather than reference order) Note that the last clarification is what distinguishes gc and gccgo at the moment: gc uses reference order (i.e., order in which variables are referenced in initialization expressions), while gccgo uses declaration order for independent variables. Not a language change. But adopting this CL will clarify what constitutes a dependency. Fixes issue 6703.

Patch Set 1 #

Patch Set 2 : diff -r 1ce8d70162aa https://code.google.com/p/go/ #

Patch Set 3 : diff -r 2c7ba8c04515 https://code.google.com/p/go/ #

Patch Set 4 : diff -r 209bb94bd691 https://code.google.com/p/go #

Patch Set 5 : diff -r 209bb94bd691 https://code.google.com/p/go #

Total comments: 10

Patch Set 6 : diff -r 209bb94bd691 https://code.google.com/p/go/ #

Patch Set 7 : diff -r 209bb94bd691 https://code.google.com/p/go/ #

Total comments: 6

Patch Set 8 : diff -r 209bb94bd691 https://code.google.com/p/go/ #

Total comments: 14

Patch Set 9 : diff -r 209bb94bd691 https://code.google.com/p/go/ #

Patch Set 10 : diff -r 209bb94bd691 https://code.google.com/p/go/ #

Total comments: 14

Patch Set 11 : diff -r 5283a606b755 https://code.google.com/p/go/ #

Patch Set 12 : diff -r 0f7c69d6c367 https://code.google.com/p/go/ #

Total comments: 2

Patch Set 13 : diff -r c9bcacf2ba33 https://code.google.com/p/go/ #

Patch Set 14 : diff -r c9bcacf2ba33 https://code.google.com/p/go/ #

Patch Set 15 : diff -r c9bcacf2ba33 https://code.google.com/p/go/ #

Patch Set 16 : diff -r c0c00145e2d3 https://code.google.com/p/go/ #

Total comments: 4

Patch Set 17 : diff -r c0c00145e2d3 https://code.google.com/p/go/ #

Total comments: 4

Patch Set 18 : diff -r c0c00145e2d3 https://code.google.com/p/go/ #

Total comments: 5

Patch Set 19 : diff -r ddbcb0e26855 https://code.google.com/p/go/ #

Patch Set 20 : diff -r ddbcb0e26855 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+122 lines, -68 lines) Patch
M doc/go_spec.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 5 chunks +122 lines, -68 lines 0 comments Download

Messages

Total messages: 30
gri
Hello r@golang.org, rsc@golang.org, iant@golang.org, ken@golang.org (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to ...
9 years, 10 months ago (2014-05-09 04:46:51 UTC) #1
adonovan
Much better. https://codereview.appspot.com/99020043/diff/80001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/80001/doc/go_spec.html#newcode4020 doc/go_spec.html:4020: At package level, <a href="#Package_initialization"</a>initialization dependencies</a> Bad ...
9 years, 10 months ago (2014-05-09 15:00:08 UTC) #2
adonovan
On 2014/05/09 15:00:08, adonovan wrote: > Much better. > > https://codereview.appspot.com/99020043/diff/80001/doc/go_spec.html > File doc/go_spec.html (right): ...
9 years, 10 months ago (2014-05-09 15:04:40 UTC) #3
adonovan
On 9 May 2014 11:04, <adonovan@google.com> wrote: > > One other thing: you should clarify ...
9 years, 10 months ago (2014-05-09 15:14:13 UTC) #4
gri
PTAL Specifically, please note that extra sentence: Specifically, the order in which variables are referenced ...
9 years, 10 months ago (2014-05-09 17:55:17 UTC) #5
gri
https://codereview.appspot.com/99020043/diff/80001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/80001/doc/go_spec.html#newcode5930 doc/go_spec.html:5930: If two variables are not interdependent, they will be ...
9 years, 10 months ago (2014-05-09 18:46:54 UTC) #6
adonovan
https://codereview.appspot.com/99020043/diff/110002/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/110002/doc/go_spec.html#newcode5890 doc/go_spec.html:5890: variables, only on the appearance of references to them ...
9 years, 10 months ago (2014-05-09 19:00:40 UTC) #7
gri
PTAL https://codereview.appspot.com/99020043/diff/110002/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/110002/doc/go_spec.html#newcode5890 doc/go_spec.html:5890: variables, only on the appearance of references to ...
9 years, 10 months ago (2014-05-09 19:12:21 UTC) #8
adonovan
On 2014/05/09 19:12:21, gri wrote: > PTAL > > https://codereview.appspot.com/99020043/diff/110002/doc/go_spec.html > File doc/go_spec.html (right): > ...
9 years, 10 months ago (2014-05-09 19:13:20 UTC) #9
iant
https://codereview.appspot.com/99020043/diff/130001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/130001/doc/go_spec.html#newcode5903 doc/go_spec.html:5903: respectively) contains an identifier denoting <code>y</code>, You've got three ...
9 years, 10 months ago (2014-05-09 19:48:34 UTC) #10
adonovan
https://codereview.appspot.com/99020043/diff/130001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/130001/doc/go_spec.html#newcode5928 doc/go_spec.html:5928: in the source, possibly in multiple files, as presented ...
9 years, 10 months ago (2014-05-09 20:02:02 UTC) #11
iant
https://codereview.appspot.com/99020043/diff/130001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/130001/doc/go_spec.html#newcode5928 doc/go_spec.html:5928: in the source, possibly in multiple files, as presented ...
9 years, 10 months ago (2014-05-09 20:15:40 UTC) #12
gri
PTAL https://codereview.appspot.com/99020043/diff/130001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/130001/doc/go_spec.html#newcode5903 doc/go_spec.html:5903: respectively) contains an identifier denoting <code>y</code>, On 2014/05/09 ...
9 years, 10 months ago (2014-05-09 20:53:20 UTC) #13
r
please let me review this before submitting. i want to have time to look it ...
9 years, 10 months ago (2014-05-09 21:15:55 UTC) #14
gri
Sure, I'm not going to submit until we're all on the same page. Just FYI, ...
9 years, 10 months ago (2014-05-09 21:24:32 UTC) #15
r
https://codereview.appspot.com/99020043/diff/160001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/160001/doc/go_spec.html#newcode5901 doc/go_spec.html:5901: An (initialization) expression, function, or method <code>x</code> depends on ...
9 years, 10 months ago (2014-05-12 19:43:08 UTC) #16
gri
PTAL https://codereview.appspot.com/99020043/diff/160001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/160001/doc/go_spec.html#newcode5901 doc/go_spec.html:5901: An (initialization) expression, function, or method <code>x</code> depends ...
9 years, 10 months ago (2014-05-12 19:55:49 UTC) #17
r
it still doesn't say precisely what "depends on" means. i could take a crack at ...
9 years, 10 months ago (2014-05-13 21:28:51 UTC) #18
gri
I think it does. That's the purpose of the 3-step recursive definition: 1) A variable ...
9 years, 10 months ago (2014-05-13 21:40:11 UTC) #19
r
https://codereview.appspot.com/99020043/diff/200001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/200001/doc/go_spec.html#newcode5890 doc/go_spec.html:5890: variables, only on references to them in the source: ...
9 years, 10 months ago (2014-05-13 22:00:09 UTC) #20
gri
PTAL https://codereview.appspot.com/99020043/diff/200001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/200001/doc/go_spec.html#newcode5890 doc/go_spec.html:5890: variables, only on references to them in the ...
9 years, 10 months ago (2014-05-14 18:15:20 UTC) #21
r
this is much better https://codereview.appspot.com/99020043/diff/280001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/280001/doc/go_spec.html#newcode5927 doc/go_spec.html:5927: thus <code>x</code> is indirectly dependent ...
9 years, 10 months ago (2014-05-14 19:24:03 UTC) #22
gri
PTAL https://codereview.appspot.com/99020043/diff/280001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/280001/doc/go_spec.html#newcode5927 doc/go_spec.html:5927: thus <code>x</code> is indirectly dependent on <code>y</code>. On ...
9 years, 10 months ago (2014-05-14 19:44:35 UTC) #23
r
LGTM but let others weigh in
9 years, 10 months ago (2014-05-14 19:59:36 UTC) #24
adonovan
LGTM https://codereview.appspot.com/99020043/diff/300001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/300001/doc/go_spec.html#newcode5941 doc/go_spec.html:5941: <code>t.m</code>, where the (static) type of <code>t</code> is ...
9 years, 10 months ago (2014-05-14 20:33:28 UTC) #25
gri
https://codereview.appspot.com/99020043/diff/300001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/300001/doc/go_spec.html#newcode5941 doc/go_spec.html:5941: <code>t.m</code>, where the (static) type of <code>t</code> is On ...
9 years, 10 months ago (2014-05-14 20:46:10 UTC) #26
iant
LGTM
9 years, 10 months ago (2014-05-14 21:40:14 UTC) #27
rsc
LGTM https://codereview.appspot.com/99020043/diff/320001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/320001/doc/go_spec.html#newcode5974 doc/go_spec.html:5974: b = 2 It might be worth strengthening ...
9 years, 10 months ago (2014-05-20 19:38:51 UTC) #28
gri
https://codereview.appspot.com/99020043/diff/320001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/99020043/diff/320001/doc/go_spec.html#newcode5974 doc/go_spec.html:5974: b = 2 On 2014/05/20 19:38:51, rsc wrote: > ...
9 years, 10 months ago (2014-05-20 20:50:47 UTC) #29
gri
9 years, 10 months ago (2014-05-20 20:51:48 UTC) #30
*** Submitted as https://code.google.com/p/go/source/detail?r=eabf83a5e11c ***

spec: clarify section on package initialization

- split description of package initialization and
  program execution
- better grouping of concerns in section on package
  initialization
- more explicit definition of what constitues a
  dependency
- removed language about constant dependencies -
  they are computed at compile-time and not
  initialized at run-time
- clarified that independent variables are initialized
  in declaration order (rather than reference order)

Note that the last clarification is what distinguishes
gc and gccgo at the moment: gc uses reference order
(i.e., order in which variables are referenced in
initialization expressions), while gccgo uses declaration
order for independent variables.

Not a language change. But adopting this CL will
clarify what constitutes a dependency.

Fixes issue 6703.

LGTM=adonovan, r, iant, rsc
R=r, rsc, iant, ken, adonovan
CC=golang-codereviews
https://codereview.appspot.com/99020043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b