Issue 7838045: code review 7838045: doc/go1.1.html: return requirements (Closed)
|
Created:
11 years, 11 months ago by r Modified:
11 years, 11 months ago Reviewers:
CC:
golang-dev, rsc, jnj, gri Visibility:
Public. |
Descriptiondoc/go1.1.html: return requirements
Patch Set 1 #
Total comments: 9
Patch Set 2 : diff -r ede73f029e7a https://code.google.com/p/go #
Total comments: 8
Patch Set 3 : diff -r 9c66fd61d54f https://code.google.com/p/go #Patch Set 4 : diff -r b0d920685dda https://code.google.com/p/go #
Total comments: 6
Patch Set 5 : diff -r b0d920685dda https://code.google.com/p/go #Patch Set 6 : diff -r f3298a9f1b89 https://code.google.com/p/go #MessagesTotal messages: 13
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
Sign in to reply to this message.
https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html File doc/go1.1.html (right): https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html#newcode58 doc/go1.1.html:58: or call to <code>panic</code> at or goto, but you're welcome not to mention that https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html#newcode67 doc/go1.1.html:67: If a function can be shown <em>syntactically</em> to have no path "If the final statement of a function can be shown ..." It's important that the definition is not about paths to the end of the function from the beginning. It only looks at the final statement. https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html#newcode68 doc/go1.1.html:68: to the end of the function, the final "return" statement is unnecessary. s/the final/no final/ https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html#newcode78 doc/go1.1.html:78: In Go 1.1, if a This paragraph and the next seem like they are repeating the previous one. Perhaps the <em>terminating statements</em> above should have the link and these this paragraph can be dropped. https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html#newcode80 doc/go1.1.html:80: The full details are spell out in the s/spell/&ed/ https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html#newcode92 doc/go1.1.html:92: be simplified manually. TODO: Such code can be identified using 'go vet'.
Sign in to reply to this message.
https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html File doc/go1.1.html (right): https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html#newcode85 doc/go1.1.html:85: Note that the rule is purly syntactic: it pays no attention to the values in the purely
Sign in to reply to this message.
FYI. But perhaps the last few paragraphs can be trimmed per rsc' suggestion. https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html File doc/go1.1.html (right): https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html#newcode85 doc/go1.1.html:85: Note that the rule is purly syntactic: it pays no attention to the values in the s/purly/purely/ https://codereview.appspot.com/7838045/diff/1/doc/go1.1.html#newcode86 doc/go1.1.html:86: code and therefore requires no semantic analysis. s/semantic/complex/ (the analysis is semantic, in my mind)
Sign in to reply to this message.
Hello golang-dev@googlegroups.com, rsc@golang.org, jeremyjackins@gmail.com, gri@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
Sign in to reply to this message.
https://codereview.appspot.com/7838045/diff/6001/doc/go1.1.html File doc/go1.1.html (right): https://codereview.appspot.com/7838045/diff/6001/doc/go1.1.html#newcode61 doc/go1.1.html:61: where a terminal "return" is clearly unnecessary, such as a function with s/terminal/final/ ? https://codereview.appspot.com/7838045/diff/6001/doc/go1.1.html#newcode66 doc/go1.1.html:66: In Go 1.1, the rule about terminal "return" statements is more permissive. s/terminal/final/ ? Below you refer to "final returns". Also, it's fewer terminating terminal thingies. https://codereview.appspot.com/7838045/diff/6001/doc/go1.1.html#newcode67 doc/go1.1.html:67: It introduces the concept of a s/concept/notion/ ? https://codereview.appspot.com/7838045/diff/6001/doc/go1.1.html#newcode73 doc/go1.1.html:73: If a function can be shown <em>syntactically</em> to have no path I think this sentence introduces again the idea of a path of control flow which I think we want to avoid: only the last statement matters. How about: If the last statement in a function can be shown <em>syntactically</em> to be a terminating statement, no final "return" is required.
Sign in to reply to this message.
Hello golang-dev@googlegroups.com, rsc@golang.org, jeremyjackins@gmail.com, gri@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
Sign in to reply to this message.
https://codereview.appspot.com/7838045/diff/6001/doc/go1.1.html File doc/go1.1.html (right): https://codereview.appspot.com/7838045/diff/6001/doc/go1.1.html#newcode61 doc/go1.1.html:61: where a terminal "return" is clearly unnecessary, such as a function with On 2013/03/22 21:39:32, gri wrote: > s/terminal/final/ ? Done. https://codereview.appspot.com/7838045/diff/6001/doc/go1.1.html#newcode66 doc/go1.1.html:66: In Go 1.1, the rule about terminal "return" statements is more permissive. On 2013/03/22 21:39:32, gri wrote: > s/terminal/final/ ? > > Below you refer to "final returns". Also, it's fewer terminating terminal > thingies. Done. https://codereview.appspot.com/7838045/diff/6001/doc/go1.1.html#newcode67 doc/go1.1.html:67: It introduces the concept of a no https://codereview.appspot.com/7838045/diff/6001/doc/go1.1.html#newcode73 doc/go1.1.html:73: If a function can be shown <em>syntactically</em> to have no path On 2013/03/22 21:39:32, gri wrote: > I think this sentence introduces again the idea of a path of control flow which > I think we want to avoid: only the last statement matters. How about: > > If the last statement in a function can be shown <em>syntactically</em> to be a > terminating statement, no final "return" is required. Done.
Sign in to reply to this message.
LGTM with at least my 2nd comment addressed https://codereview.appspot.com/7838045/diff/14001/doc/go1.1.html File doc/go1.1.html (right): https://codereview.appspot.com/7838045/diff/14001/doc/go1.1.html#newcode62 doc/go1.1.html:62: only an infinite "for" loop. fine for this document but really it's only for loops w/o a condition. for true {} is infinite but not terminating per the spec. Also, it's mentioned again below. Maybe just: But there are many cases where a final "return" is clearly unnecessary. https://codereview.appspot.com/7838045/diff/14001/doc/go1.1.html#newcode74 doc/go1.1.html:74: be a terminating statement, no final "return" statement is unnecessary. s/unnecessary/necessary/.
Sign in to reply to this message.
LGTM https://codereview.appspot.com/7838045/diff/14001/doc/go1.1.html File doc/go1.1.html (right): https://codereview.appspot.com/7838045/diff/14001/doc/go1.1.html#newcode74 doc/go1.1.html:74: be a terminating statement, no final "return" statement is unnecessary. s/unnecessary/necessary/ (or needed) https://codereview.appspot.com/7838045/diff/14001/doc/go1.1.html#newcode86 doc/go1.1.html:86: TODO: Such code can be identified by <code>go vet</code>. vet is in
Sign in to reply to this message.
https://codereview.appspot.com/7838045/diff/14001/doc/go1.1.html File doc/go1.1.html (right): https://codereview.appspot.com/7838045/diff/14001/doc/go1.1.html#newcode62 doc/go1.1.html:62: only an infinite "for" loop. i think at this level of explanation the example is fine. this isn't the spec and doesn't need to be precise or all-inclusive. https://codereview.appspot.com/7838045/diff/14001/doc/go1.1.html#newcode74 doc/go1.1.html:74: be a terminating statement, no final "return" statement is unnecessary. On 2013/03/22 21:48:24, gri wrote: > s/unnecessary/necessary/. Done.
Sign in to reply to this message.
Hello golang-dev@googlegroups.com, rsc@golang.org, jeremyjackins@gmail.com, gri@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
Sign in to reply to this message.
*** Submitted as https://code.google.com/p/go/source/detail?r=61dd0f95b763 *** doc/go1.1.html: return requirements R=golang-dev, rsc, jeremyjackins, gri CC=golang-dev https://codereview.appspot.com/7838045
Sign in to reply to this message.
|
