proposal: spec: permit goto over declaration if variable is not used after goto label

[pjebs]

Right now you get this error: goto SKIP jumps over declaration of queryArgs

if len(s.standardDays) == 0 {
goto SKIP
}

queryArgs := []interface{}{}
//Do stuff with queryArgs

SKIP:
//Do stuff here BUT queryArgs is not used ANYWHERE from SKIP onwards

Currently I have to refactor all my code like this (including placing var err error at the top):

var queryArgs []interface{}

if len(s.standardDays) == 0 {
goto SKIP
}

queryArgs = []interface{}{}
//Do stuff with queryArgs

SKIP:
//Do stuff here BUT queryArgs is not used ANYWHERE from SKIP onwards

[pjebs]

This can be fixed in Go 1 too since it's backwards compatible.

[ianlancetaylor]

This seems like it makes code a bit fragile: if you do this, and then later add a use of the variable, you will get a compilation error about lines that you didn't touch at all.

What is the benefit of making this change? How often does this problem arise?

[pjebs]

The benefit is that the goto does what intuitively it is meant to do.

I can't tell you how often the problem arises because the use of goto is extremely rare ..... but when I do attempt to use goto, it arrises VERY VERY often and it usually means I have to refactor my code to place the variable definitions at the top (above the goto), rather than have the variables where they are most appropriate, and also I lose the ability to use := for variable define+assign.

Re:
if you do this, and then later add a use of the variable, you will get a compilation error about lines that you didn't touch at all.

Of the best parts of Go is how amazing and helpful the compiler is. If I later add a use of the variable, I would want to get a compilation error. It usually means that goto is no longer the best way to write the code.

[mark-rushakoff]

Instead of moving variable declarations to the top, you could just introduce a new block around what you're goto-ing over:

package main

import (
	"fmt"
)

func main() {
	if true {
		goto FINISH
	}

	// New block, so it's safe to skip the declaration and assignment of secret.
	{
		secret := "You'll never see this line."
		fmt.Println(secret)
	}

FINISH:
	fmt.Println("Bye!")
}

https://play.golang.org/p/kzFxyaeOmkQ

[cznic]

The status quo enables optimizations that would be no more [easily] possible under the proposal.

[mark-rushakoff]

Looks like this could be closed as a duplicate of #27165.

[ianlancetaylor]

This is a slightly different approach to the same problem.

[ianlancetaylor]

CC @griesemer

[griesemer]

I believe the intent of this proposal is the same as in #27165 (with that proposal based on an idea discussed in May of this year at an internal Go Team summit, but only written down yesterday).

However, this proposal disallows a goto if it jumps over a variable declaration of a variable that is used (lexically) after the target label (but always allows the variable access). In contrast, proposal #27165 disallows access of a variable (lexically) after a label, if there is a goto that jumped over it's declaration (but always allows the goto).

It does not seem immediately clear which approach is better. I think we should leave this proposal open as a clearly viable alternative.

[pjebs]

My proposal is much clearer to understand in my opinion. The alternative proposal will be confusing for beginners.

It is also Go 1 compatible. I don't think the alternative is.

[ianlancetaylor]

This is what I said over on #27165:

Closing in favor of #26058, which is the same idea expressed in a slightly different way. Both issues accept the same set of programs; the difference is whether the error is reported on the use of the variable or on the goto statement. The (small) advantage of #26058 is that it does not change variable scope. It just relaxes, slightly, the existing constraint on goto over a variable declaration.

(I do still think that both proposals accept the same set of programs. Might be interesting to see a program that behaves differently under the two proposals. Not very important, though.)

[pjebs]

will it be applied to Go 1 too?

[ianlancetaylor]

We aren't making any language changes until the Go 2 process starts.

Note that this has not been accepted.

[ianlancetaylor]

To clarify, it hasn't been declined, either. It's pending a decision.

[darkfeline]

Isn't this proposal just syntactic sugar for @mark-rushakoff's suggestion? In which case the decision to accept this proposal boils down to whether we want such syntactic sugar. Consider:

if foo {
	goto done
}
x := 1
if bar {
	goto done
}
y := 1
if baz {
	goto done
}
z := 1
fmt.Println(x, y, z)
done:
doStuff()

Under this proposal, this is exactly equivalent to:

if foo {
	goto done
}
{
   	x := 1
	if bar {
		goto done
	}
        {
        	y := 1
		if baz {
			goto done
		}
                {
			z := 1
        		fmt.Println(x, y, z)
                }
        }
}
done:
doStuff()

https://play.golang.org/p/4W6wRO-rmel

The syntactic sugar would very desirable in such an example.

[rsc]

The current goto rules balance two competing goals:
(1) don't allow valid Go programs refer to uninitialized variables, and
(2) don't require complex analysis to decide whether a Go program is valid.

@pjebs's proposal and @mark-rushakoff's rewrite are different in general, although perhaps not in this case, and the difference highlights exactly how we have balanced these two competing goals.

It is true that this invalid Go program would, if it were redefined to be valid, not refer to uninitialized variables:

goto L
x := 1
print(x)
L:

It is also true that it can be mechanically transformed into this valid Go program:

goto L
{
x := 1
print(x)
}
L:

(The only change is the introduction of { and } on two new lines.)
However, here is an invalid Go program that does refer to uninitialized variables:

goto L
x := 1
L2:
print(x)
goto L3
L:
goto L2
L3:

The mechanical block rewrite preserves invalidity, because the goto L2 cannot jump into a block:

goto L
{
x := 1
L2:
print(x)
goto L3
}
L:
goto L2
L3:

(Again, the only change is the introduction of { and } on two new lines.)

It is of course good that adding braces has not taken this bad program from invalid to valid. That is why the block restriction exists.

The original message on this proposal did not suggest a detailed fix (the original title was simply "fix goto"). It has been retitled "permit goto over declaration if variable is not used after goto label" but a literal reading of "after" meaning "after in the source code" would violate goal (1) by admitting this buggy program. A reading of "after" meaning "after in the control flow" would violate goal (2) by requiring more complex compiler analysis to decide whether a program is valid.

---

Obviously, C compilers do this kind of analysis in their "used and not set" warnings, but experience has shown that these are sensitive to how aggressively the compiler optimizes (= learns new details about) the code. Whether a particular input is a valid Go program must not depend on compiler optimization level. There needs to be a clear, simple process for deciding. That process must produce consistent results in all implementations, and it must not admit any buggy programs.

As an example of where we have solved this kind of problem in the past, functions with results originally had to end in a return statement, but that was problematic for things like

func max(x, y int) int {
    if x > y {
        return x
    } else {
        return y
    } 
    // invalid Go program: missing return - in early versions of Go
}

We fixed this by writing a precise definition of a terminating statement and requiring that functions end with one of those instead of just a return.

We could conceivably do something equally precise here. But I think the definition would end up being longer, since it would have to capture some clear algorithm for deciding control flow. The current rules avoid introducing a control flow algorithm into the spec, because of goal (2).

---

I do see one possible simple change, which would be to say that it's ok to jump over declarations (unconditionally), and that jumping over one is the same as executing it, meaning the name is in scope and has the zero value. Goal (1) is satisfied because the programs never see uninitialized variables, and goal (2) is satisfied because there is no complex logic about whether a program is valid. An optimizing compiler would need to initialize the value earlier than it comes into scope, by sliding any (zeroing) initialization code backward up to just before the first goto that might jump over the value. This is basically what @pjebs says he does by hand already in the top comment.

Is this worth doing? Maybe, maybe not. That's an open question.

[ianlancetaylor]

For what it's worth, I think another way to achieve the goal of this proposal would be to say that goto label is permitted to skip over a variable declaration if 1) label is in the outermost block of the function; 2) label is not followed by a goto statement; 3) the variable is not referenced after label. But I don't know if it's worth it.

[rsc]

For what it's worth, I don't understand why the outermost block would need to be called out specially to keep the program valid. Maybe I am missing something, of course.

I realize I should have said one more thing about the "Is this worth doing? Maybe, maybe not." Ultimately that comes down to first finding the simplest viable solution we can and then deciding whether the cost of adopting that solution is paid for by the benefits it would bring. In this case the benefits seem very near zero: goto is in the language primarily for generated code, and even for hand-written code it is trivial to move the necessary declarations earlier and keeps the code clear. On the other side of the scale, rolling out any change to language semantics has real, non-zero costs in terms of implementations and documentation (not just in our repo but everything everyone has ever written about goto). It's not obvious to me that the benefit of improvements here could possibly outweigh those fixed costs inherent to any change at all.

[ianlancetaylor]

You're right, we don't need the outermost block restriction. I got confused about continue L. But that shouldn't be a problem because a continue statement after a goto can only ever get us back before the goto.

[josharian]

goto is in the language primarily for generated code, and even for hand-written code it is trivial to move the necessary declarations earlier and keeps the code clear.

Anecdotally, this restriction is extremely frustrating when working by hand on non-trivial functions involving gotos. My experience is with trying to clean up and clarify the compiler after the c2go rewrite. Being able to shorten variable lifetime is an important step in making code clear. I often found that I had to refactor to remove gotos, even in cases in which they did not otherwise impede readability, before I could do any other meaningful cleanup.

[pjebs]

Notwithstanding @josharian experience, from my experience, it has been a minor annoyance, but nothing too monumental to overcome.

The proposal provides benefits that are intangible:

1. goto will behave as it intuitively should. This is good for beginners of Go. I feel it makes the language more internally consistent too but that's prob a Rob Pike Q.
2. I feel that documentation will in fact be simpler. My proposal is the more natural interpretation of how it works. There is less to document in terms of the "exceptions" and "provisos" on how goto operates as is the case now.

[kortschak]

Having spent a fair bit of time porting old numerical Fortran code to Go, I'd seen a lot of GOTO use and abuse. Of the problems that come from use of gotos, this is by far and away the least problematic, the more significant issues have been (at least in those code bases) non-nesting of loops (or at least nesting of loops that are not trivially nestable) and identifying labels. Hoisting has always been a relatively easy process to work around the restriction here (and the restriction is a good motivation to remove as many goto statements as possible).

[bradfitz]

FWIW, I hit this issue today when deleting a bunch of code:

https://go-review.googlesource.com/c/go/+/199499/4/src/cmd/link/internal/ld/elf.go

Instead of just deleting the if statement and unindenting, I had to leave the otherwise unnecessary braces in because there was a goto jumping over this code.

[zigo101]

How about using this old idea to mute (terminate the life of) a variable in a scope explicitly and manually?

[zigo101]

Sorry, I forgot the link: #17389

Example:

if len(s.standardDays) == 0 {
    goto SKIP
}

// ... tons of lines code

var queryArgs = []interface{}{}

//Do stuff with queryArgs

queryArgs = _ // undeclare queryArgs

SKIP: // Now this label is out of queryArgs's scope
//Do stuff here

[mikeschinkel]

Hi @ianlancetaylor:

Thank you for redirecting me from the golang-nuts mailing list to here when I asked about this topic.

How often does this problem arise?

I am sure it differs for different developers but I have found the goto <label> for early exit where there is a need to clean up before return being very a helpful pattern for numerous reasons so I use it frequently. Thus the problem arises in many of the functions I write.

Here are numerous examples from a side project I am working on, and these are only a few of many:

1. Vars declared at top — wpfr.FileProcessor.Process()
2. Vars declared at top — wpfr.Parser.GetNextToken()
3. Vars declared at top — cmpsql.BuffferedFile.Scan()
4. Vars declared at top — phpcereal.Parser.ScanPHPOctal()

As as aside, I see this use-case for using goto as being a pattern simulated with goto and a label since Go does not have an explicit syntax/feature to allow implementing this pattern, much like in the very early days of programming where developers used goto to simulate branches and loops.

I would actually prefer a bespoke syntax for this use-case, but I would still be ecstatic if the limits on goto, labels and variable declarations were relaxed as proposed above since I do not expect we'd see a bespoke syntax anytime soon, if ever.

-Mike
P.S. Is your removal of the NeedsDecision tag 5 days ago serendipitous for my needs here? Does that mean the team will soon consider addressing feature request?

[ianlancetaylor]

@mikeschinkel Honestly it seems to me that your examples can be handled more idiomatically using defer with named result parameters. Or by simply replacing the goto statement with a return statement.

The change to the NeedsDecision tag is just a coincidence.

[mikeschinkel]

@ianlancetaylor — 

Thank you for reviewing the examples and commenting.

I understand your point about defer and generally about replacing goto with return, however using this pattern has several benefits over those approaches.

defer complexity

First, I find it harder to reason about using defer because:

1. the extra visual noise required when using an inline closure with defer, and
2. I find it easier to reason about code that runs at the end of a func to be at the bottom of that func.

I guess I could put a defer at the end of the func, but then it would not be very idiomatic, and it would still be noisy. Right?

returns vs. breakpoints

As for using return, the initial reason I started using this pattern was to avoid having numerous places to breakpoint to stop a function for variable inspection. And early return does not support running code at the end, unless I use defer which brings me back to the earlier points.

repeated return values

Then as I started having to maintain other people's Go code — where others are returning three, four, five or more values (yes, that horrifies me too, but that is what I am finding almost everywhere I have been working) — so I found this pattern works better than using naked returns or repeating multiple return values in every place where I want to gracefully wrap up and exit.

benefits discovered after use

But let's assume none of that of that sounds compelling to you. After using the pattern for a while — and that is key, I do not think you can appreciate the following unless you try using it for a while — I discovered two (2) very happy benefits.

1. Using the pattern consistently in a codebase makes refactoring much easier than using defer and/or early return. When using those I often have to edit the potentially break the logic, but with this goto pattern refactoring large functions into smaller functions or vice versa becomes trivial cut & paste with much less chance of introducing logic errors.

2. When combined with happy path programming and minimization of indention this pattern makes it obvious when to split a func into multiple smaller funcs. Prior to discovering this pattern it was never very obvious to me when a func should be split, but with this pattern funcs that need to be refactored into smaller stand out like a clown at a board meeting.

In summary I have found using this pattern makes code more robust, more maintainable, and easier to reason about then when I previously used early returns. After discovering something that works so very well it feels like an epiphany it is painful to revert back to the old way.

dx

So I am not asking for this to become the new idiom for Go — although I would certainly not hate it if that came to pass — I am just asking in hopes the team will consider improving developer experience for this pattern. And since it has been mentioned as a concern but several current and past team members I assume I am not the only one who would see this proposal as an improvement?

-Mike

defer redux

P.S. One more thing about defer. Almost everything else about Go is minimal and elegant yet defer feels the opposite. I do use defer for releasing items that must be released, but for general cleanup code needed in many funcs using defer feels like it adds too much complexity.

I really hope you can empathize why I prefer not to use early return, nor defer except for use-cases where it would be irresponsible not to.