spec: define when return is necessary #65
Labels
Milestone
Comments
|
Comment 6 by ibw@isaacwagner.me: Though if you look through the Effective Go documentation, it is suggested that you
don't code with those "guaranteed" else's. I agree that it should be resolved, but I
think that leaving it the way it is may help everyone get into the habit of coding
func example(x int) int {
if x == 0 {
return 5;
}
return x;
}
as opposed to
func example(x int) int {
if x == 0 {
return 5;
} else {
return x;
}
}
I personally think the first one is cleaner and more elegant.
|
#6 may satisfy a certain taste, but I would be more concerned about anyone coming to read the code later. In toy examples this is not of much concern, but if this were a big function, that the if statement returns before the rest of the body would be less than obvious. Some poor maintainer reading this elegant code will have to check everything in the if statement, when it would be better if they could just forget it and fix what needs to be fixed. I hope #4 has been noticed. |
For the log: this is a well-known issue we plan to address, but there are higher-priority issues right now. There's no need to try to persuade us that it's a problem. For what it's worth, #6 is certainly the preferred style for that particular code, but the problem comes up in loops that don't break, etc. Especially in a big function, having the if statement return early on helps keep the code in the function from creeping to the right. |
|
Issue #1679 has been merged into this issue. |
Actually, the spec appears not to define what the behavior here is. Owner changed to @griesemer. |
|
Issue #2034 has been merged into this issue. |
Won't you have to check for unused code blocks in the optimizer stage anyways?
So why not delay the return check to the optimizer stage and deal with it there?
Take this modified example, which also outputs the same return error in GoLang:
func example(x int) int {
return 19; //Good for debugging, bad if one forgets about it
//But should the following code be included in the final binary since it's unreachable?
//If the answer is to exclude the code, then it's more of a reason to postpone return
checking to the optimizer stage
if x == 0 {
return 5;
} else {
return x;
}
}
First when generating the intermediate code for each function, automatically append a
return at the end of the function upon hitting the last }.
Then in the optimizing stage, for every intermediate opcode have a flag denoting
traversed or not traversed,
and run through the code taking both paths. If a branch tries to go to an already
traversed location, hits a return, or break, then the end of that path has been found.
Let's assume the intermediate code looks like so:
function example
..input x int
..output t0 int
..t0 = 0 #Spec states function returns are initiated to 0 on function entry
..t0 = 19 #Carry out our return (obviously the t0 = 0 would get ditched once optimized)
..return
..cmp x, 0
..bne label0
..t0 = 5
..return
..bra label1 #Automatically added by if/else block generation - but rendered useless
because of the "return" above
label0:
..t0 = x
..return
label1:
..return #The automatically appended return at end of function, done by the intermediate
code generation stage
And once we run through it:
function example
..input x int
..output t0 int
..t0 = 0 (Traversed)
..t0 = 19 (Traversed)
..return (Traversed)
..cmp x, 0 (Untraversed)
..bne label0 (Untraversed)
..t0 = 5 (Untraversed)
..return (Untraversed)
..bra label1 (Untraversed)
label0:
..t0 = x (Untraversed)
..return (Untraversed)
label1:
..return (Untraversed)
Then we drop all the untraversed lines and end up with this:
function example
..input x int
..output t0 int
..t0 = 0 (Traversed) #Once the symbols are optomized, this line should be dropped
obviously
..t0 = 19 (Traversed)
..return (Traversed) #Return is last
But let's say the function was this:
func example(x int) int {
if x == 0 {
return 5;
} else {
return x;
}
}
So the intermediate code would be this:
function example
..input x int
..output t0 int
..t0 = 0 #Spec states function returns are initiated to 0 on function entry
..cmp x, 0
..bne label0
..t0 = 5
..return
..bra label1 #Automatically added by if/else block generation - but rendered useless
because of the "return" above
label0:
..t0 = x
..return
label1:
..return #The automatically appended return at end of function, done by the intermediate
code generation stage
Then in the optimizing stage, when traversing we get:
function example
..input x int
..output t0 int
..t0 = 0 (Traversed 1)
..cmp x, 0 (Traversed 2 )
..bne label0 (Traversed 3) #both branches taken
..t0 = 5 (Traversed 3.a.1)
..return (Traversed 3.a.2)
..bra label1 (Untraversed)
label0:
..t0 = x (Traversed 3.b.1)
..return (Traversed 3.b.1)
label1:
..return (Untraversed)
Drop the untraversed lines:
function example
..input x int
..output t0 int
..t0 = 0 (Traversed 1)
..cmp x, 0 (Traversed 2 )
..bne label0 (Traversed 3) #both branches taken
..t0 = 5 (Traversed 3.a.1)
..return (Traversed 3.a.2)
label0:
..t0 = x (Traversed 3.b.1)
..return (Traversed 3.b.1) #Return is last, okay
Furthermore, I think the compiler should not treat this as an full-stop-error, but
instead as a warning.
The spec already claims all return values are declared to 0 upon entry of the function.
So the lack of an explicit return only implies a defined state - a return of 0
|
The goal here is to maintain consistency across the compilers. If you delay return checking to an "optimizer stage", then you inherently get different compile errors depending on your optimization settings, and that's not consistent. The rule here is trivial: the function must end with a return statement or a panic statement or have no return values. |
|
Issue #3979 has been merged into this issue. |
I agree with #14.
Honestly, I find it frustrating that the compiler stops due to this error. This, as
well as the unused variable errors, should not be errors. The compiler should be smart
enough to strip these out and compile the program anyway, since it makes exploratory
debugging much easier. There is really no reason for the compiler to be so picky. It
can issue warnings.
And it full on stops even in situations when it is wrong, like this one. That's
especially frustrating.
I find this code to be more consistent and readable than the other:
if x == 0 {
return 5;
} else {
return x;
}
|
Somebody said that style
if x == 0 {
return 5
}
return x
is more safe and elegant, but I don't agree, I think with `else` I can:
be sure that control from `true` branch never reaches the `false` branch and also is
more natural. How do you read this code? "return 5 if x == 0 otherwise x" and not like
"return 5 if x == 0 return 0":) Please fix it.
|
The current plan is http://golang.org/s/go11return. |
|
This issue was closed by revision 9905cec. Status changed to Fixed. |
|
This issue was closed by revision ecab408. |
minux
pushed a commit
to minux/goios
that referenced
this issue
Feb 27, 2015
This change adds the C_VCONADDR representing $r(SB), the address of a relocatable symbol. The old C_ADDR, r(SB) without $, means data at a relocatable symbol. C_VCONADDR matches C_VCON in optab. We could have overloaded the meaning of C_VCON, but adding a new class simplifies the logic in addpool. Fixes golang#65
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
by bob.appleyard:
func example(x int) int { if x == 0 { return 5; } else { return x; } } Gives the error "function ends without a return statement" Changing it to: func example(x int) int { if x == 0 { return 5; } else { return x; } panic("unreachable"); } Solves the problem, however this is quite clearly a workaround. GOOS=linux GOARCH=amd64 Local revision: changeset: 3988:b773b8255a8f tag: tip user: Russ Cox <rsc@golang.org> date: Wed Nov 11 13:08:35 2009 -0800 summary: avoid clash with stdio's getc, ungetc.The text was updated successfully, but these errors were encountered: