Thank you very much @jaredpar for sharing this perspective!

@jaredpar Thank you very much! That was very informative and insightful.

Do you have any further perspective to share on the decision to change foreach but not for? Was it considered an option to change for at the time? Has there been any discussion since about changing for? (Particularly since ES6 decided to use per-iteration bindings for 3-clause for(let loops.)

Internally within the Go compiler team, this issue has recurringly been discussed as: (1) Go's for/range loop semantics are unfortunate and error-prone; (2) it would also be unfortunate to change for/range loops without changing (non-range) for loops too; but (3) changing for loops is much more controversial still.

The compiler team has been leaning towards changing both, mostly motivated to make sure users can safely switch between for and for/range without having to think about variable binding semantics. But that's not yet a unanimously held position.

changing [non-range] for loops is much more controversial still.

For what it's worth, i'm satisfied with the answers you and Russ provided elsethread. As long as the solution can be narrowly tailored to fix the closure-in-a-loop problem without otherwise changing for loop semantics (which it sounds like it can) then i'm happy.

Do you have any further perspective to share on the decision to change foreach but not for? Was it considered an option to change for at the time

It was considered but rejected.

One issue is that foreach was primarily a C# syntax at the time where for is a more general language syntax. The for loop in C# is really no different than for in C/C++ and a variety of other languages. Changing the semantics of foreach was asking developers to adjust how they thought about C#. But changing the semantics of for was asking developers to think differently about C# than they did for most of the other languages that they used. That's a dicey proposition.

The other thought was that in for loops it was much clearer what the lifetime was. There are explicit declaration with initializations, the conditional check and increment. Taken together the feeling was it was more obvious that the lifetime was for the entire loop. Anecdotally that matches the data I've seen in terms of bug reports over the years.

Has there been any discussion since about changing for? (Particularly since ES6 decided to use per-iteration bindings for 3-clause for(let loops.)

We haven't had any serious discussions about it that I remember.

A bit of context that may not be as obvious from the outside. At that time the C# team was extremely sensitive to breaking changes. Given we couldn't leverage <LangVersion> any toolset upgrade meant all customers saw all breaking changes. Even small breaks could result in significant push back from customers who just wanted to upgrade the tools. As such the team went to great lengths to avoid them whenever possible. Changing foreach was one of the biggest breaking change the language had really taken on. Going after for at the same time likely would've been a bridge too far. If we were approach the same discussion today I suspect there would be a stronger desire to change both.

Are there any valid serious use cases preferring the current semantics? If there are none, the C# way is better than the go.mod way in my opinion. Most code readers don't care about the go version set in go.mod at all. The fact that the same piece of code behaves differently is a bad user experience and a potential security risk for many projects.

Yes, we kept the implied version for an absent go line / go.mod file bumping forward for as long as the releases were completely compatible (or close enough). That stopped at Go 1.17, as you note. Since then, all releases assume that code without a go line gets Go 1.16 semantics. That is set in stone now and will never change.

This exactly matches my experience. It's relatively easy to understand the first example (taking the same address each time), but somewhat trickier to understand in the closure/goroutine case. And even when you do understand it, one forgets (apparently even Russ forgets!). In addition, issues with this often don't show up right away, and then when debugging an issue, I find it always takes a while to realize that it's "that old loop variable issue again".

I also find there to be a frustrating split of “workarounds” for closures. Some people recommend redefinition, and others assignment through a function parameter:

for _, thing := range things {
  thing := thing // explanation for people who don’t understand the pattern
  go func() {
    do(thing)
  }()
}

vs.

for _, thing := range things {
  go func(thing Thing) {
    do(thing)
  }(thing)
}

I’ve tried to explain that the first option, while seemingly non-functional, is the better option: it uses type inference so a change in type name does not require a change in this loop; it shadows the per-loop thing variable meaning you cannot accidentally reference the per-loop value; definition and assignment precede use. While the latter additionally requires: going to the end of the loop to verify that the assumption that per-loop thing is assigned as the argument for the parameter thing in the anonymous function (that is, definition precedes usage, but usage precedes assignment); there is a tendency of people to dislike shadowing variable names, which leads them to using a different name for the parameter from the per-loop variable, which leaves the per-loop variable exposed and referenceable.

In the end, both patterns “solve” the problem, but neither of them in a wholly satisfactory way. The per-iteration redefinition provides the most concise and robust code, but really calls for documentation every time because maybe this is the first time a person has come across this pattern, and might think it superfluous.

All in all, I’m pretty happy to see this semantic change. Work around for more efficient only-once allocation being a welcome burden for not having to explain this extremely common bug every time it pops up. The only-once allocation being far more clear as a result as well, which doesn’t end up looking unnecessary:

var thing Thing // this is _obviously_ not per-iteration allocation, and requires no explanation
for _, thing = range things {
  do(thing)
}

@DeedleFake This discussion is only about changing loops that explicitly declare variables. Loops that do not declare variables, like your example, are unaffected

Oh, duh. After all, without a declaration, how would the compiler know which variable to copy and update on each iteration?

I think the argument in #56010 (reply in thread) is a stronger reason to prefer leaving 3-argument for as-is, than the correctness arguments here.

It seems that the 3-argument for loop construction looking identical to many other languages, but being in fact slightly different, would be something difficult to teach and difficult for new learners to retain (since it generally 'just works' regardless).

the 3-argument for loop construction looking identical to many other languages, but being in fact slightly different, would be something difficult to teach and difficult for new learners to retain (since it generally 'just works' regardless).

So would the 3 argument for loop looking similar to the range loop and working slightly differently

The proposal that I read (unless rsc updated it between your comment and me reading it), is clear that in the 3 argument for loop, the per-iteration variable’s value would be copied into a per-loop variable prior to operation of the 3rd argument.

Thus, your presented code should still work fine:

for i := 0; i < len(a); i++ {
   if a[i] == "" {
      a = append(a[:i], a[i+1:]...)
      i-- // the value of the per-iteration loop set here will be copied into the per-loop `i` prior to the `i++`
   }
}

For modules at go 1.17 or higher, go mod vendor preserves the go versions for dependencies (see https://go.dev/doc/go1.17#vendor / #36876).

Thanks, @bcmills. I thought that was the case, but couldn't find it. I realize now that the reason I couldn't find it in modules.txt is that I happened to use a dependency that isn't a Go module.

How would loop variables be treated in those kinds of cases?

Un-Go-versioned dependencies are treated as being written for Go 1.16 already for certain module analyses. The same would apply here.

Any message that does not break the build process is going to be overlooked by at least some people and all automation. There’s kind of the idea in Go that if something is worth warning about, then it’s worth breaking the build/automation.

That would mean we would want to stop any go mod edit that tries to advance past 1.30 in the prescribed instances.

This is what tests are for, not obscure warning messages to the final user that they then must go spelunking about because there might be a problem somewhere.

We can definitely provide tools for people to analyze their code to understand likely sources of changes, and we would absolutely do that.

I'm not sure it makes sense to track "was this code ever compiled with Go 1.30 before?" and print information during an ordinary build. That question can't be answered reliably (what if the upgrade happened on a different machine and was checked into the repo and you just ran git pull?).

I'm not sure it makes sense to track "was this code ever compiled with Go 1.30 before?"

That's true. Thinking a bit more about this, maybe a (standalone?) Go tool that takes the compiler, with all its flags and, for a given codebase, runs all the Go versions between "current" and "target" and produces the following output:
Upgrading from current (Go 1.15) to target (Go 1.18) will result in the following changes:

• Change 1 - file:line caused by since Go 1.17
• Change 2 - file:line caused by since Go 1.16
• Change 3 - file:line caused by since Go 1.18
• ...

The tool could keep track of more than just compiler changes. Deprecations, library behavior changes for functions, etc, could all be part of this.

Then the developers could run this tool and provide a clear upgrade path. This would make it easy to determine if/what work is required for developers to upgrade a codebase.

Edit: Integration with editors/other tools would then help people discover these issues easier.

1. The code looked like, roughly:

   var once sync.Once
   for _, tt := range testCases {
       once.Do(func() {
           http.HandleFunc("/handler", func(w http.ResponseWriter, r *http.Request) {
               w.Write(tt.Body)
           })
       })
       
       result := get("/handler")
       if result != tt.Body {
           ...
       }
   }
   

   It took us a while to decode this too, and in fact the original was not as clear. (Not all code in the world is well written!)

2. If you compile your package with -gcflags=-m and see a moved to heap: i message about your loop variable, those will be allocated once per iteration instead of once per loop. We don't have a diagnostic for the case where a variable escapes from the loop but not the function. For example:

    % cat /tmp/x.go
    package p
    
    var global *int
    
    func f(x []string) {
    	var last *string
    	for i, v := range x {
    		if len(v)%3 == 0 {
    			global = &i
    			last = &v
    		}
    	}
    	println(*last)
    }
    % go build -gcflags=-m /tmp/x.go
    # command-line-arguments
    /tmp/x.go:5:6: can inline f
    /tmp/x.go:5:8: x does not escape
    /tmp/x.go:7:6: moved to heap: i
    % 
   

   In this program, in the new semantics both i and v would need to be heap allocated once per iteration: i because its address escapes to a global, and v because its address escapes outside the loop context (to last). Today i is allocated once per loop, while v is not allocated at all.

   As part of the process (in advance of whatever release made the change) we would provide a tool that gives a more accurate accounting of the places where allocation behavior (and semantics, same places) would change.

The once.Do example would have been better written by hoisting the shared variable declarations outside the loop and doing the stubbing once, there. I thought it was very confusing code.

With the current implementation, if there is no apparent capture, the loop is not changed. I say "apparent" because the detector has to run before escape analysis and thus it is over-conservative; it looks for mention within closures and address-taking (address-taking also occurs when passing a V to a *V method or constructing a method value). I can see how in the future we'll have a discussion about "technical debt in the compiler" because its internal representation of for/for-range will have old-style capture, but doing it this way ensures usual-case no-overhead and makes it easier to detect where the change occurs.

If there is apparent capture in for-range, but escape analysis determines there is no escape, then it is very nearly the same loop.

If there is escape, then there will tend to be a new allocation for each iteration, and if that is bad, hoist the declaration prior to the loop and use "=" in the range, as in:

var i int
var v Value
for i, v = range whateverYieldsValues {
  ...
}

but escape analysis works well.

3-clause for loops with capture are much rarer (97% less common than range loops, over non-test Google code, and they didn't show up in tests at all), but the same treatment of escaping variables applies. The transformation we copied from JavaScript introduces a little branchy overhead that might not come out in optimization (but we might target that optimization in the future), but (1) if there is no increment clause, it can be omitted (that's already implemented) and (2) the same declaration-hoisting change that works for for-range also eliminates the 3-clause per-iteration variable, and thus eliminates the transformation and thus eliminates the branchy code.

So the workaround is not hard in either case.

There's also the possibility of a tool that could, if this change causes failure and you can't easily figure out where, pinpoint exactly the function and loop where it goes wrong, if we could figure out the right packaging for the tool. This would use the GOSSAHASH compiler debugging technique combined with automated binary search (that program is already written), specialized to just this problem. It would probably be designed to work with "go test".

var once sync.Once
for _, tt := range testCases {
    once.Do(func() {
        http.HandleFunc("/handler", func(w http.ResponseWriter, r *http.Request) {
            w.Write(tt.Body)
        })
    })
    
    result := get("/handler")
    if result != tt.Body {
        ...
    }
}

@rsc could you please explain this? w.Write(tt.Body) implies that tt.Body is []byte. Then equality check if result != tt.Body { should not compile? Thank you.

Sorry for late (and possibly already answered) question.

@aklepatc I assume that's an error introduced by the reproduction from memory. You can either replace != with !bytes.Equal or w.Write(tt.Body) with io.WriteString(w, tt.Body) to fix it.

The explanation of that code is that when tt.Body is used in the once.Do, it closes by reference - tt is always an implicit pointer to the single loop variable tt. If you run this sequentially, get thus returns the value of tt.Body in that run of the loop, not the first one where the handler was registered. Thus, the test works. But it is far too clever for its own good (or, more likely, accidentally correct).

A cleaner way to do that would be

var body string
http.HandleFunc("/handler", func(w http.ResponseWriter, r *http.Request) {
    io.WriteString(w, body)
})
for _, tt := range testCases {
    body = tt.Body
    result := get("/handler")
    if result != tt.Body {
        ...
    }
}

Ty @Merovius ! Bytes equality check was the only thing that didn't make sense about this code snippet. The rest was reasonably clear.

More specifically, in the example that hypothetically assume the change is made in go 1.30, I think any attempt to compile or import the work module (with go 1.30 in its go.mod) with go 1.29 or older should report errors.

I think we will want authors of modules to be able to upgrade their module to 1.30 without breaking their users (my understanding of forward compatibility). I do not feel we should consider this a breaking API change as it is an internal implementation detail. @rsc discussed forward compatible changes in #55092.

It could be reasonable for tooling to start warning about "may escape for/for range vars" once you have a mixed version workspace. The basis would be that it is a readability challenge to switch back and forth while navigating code.

And non-module based build systems (e.g. bazel) will also need a plan to move forward.

To facilitate upgrading from python 2 to 3, bazel has a python_version field for pybinary and srcs_version for pylibrary. Something similar can be added to go_library to facilitate large scale bazel migrations.

This is discussed in #55092

No matter how the effective toolchain is specified, it would become a build error to attempt to use an old toolchain with new code.

Yes, I believe shipping #55092 first would make it safer to do this change.

@rsc: reading through that proposal, it sounds like it would only upgrade the toolchain to the Go version of the main module of the project.

If the installed toolchain is Go 1.X, the main module's go.mod says go 1.Y, and a dependency module's go.mod says go 1.Z with X < Y < Z, would this work correctly if the loop variable changes are introduced in Go 1.Z?

This question puzzles me as well, and I can't think of a way to make it work without upgrading the toolchain from 1.X to 1.Z (which may not be possible). Go 1.X can't know about the semantic change in 1.Z, so it couldn't even emit a warning other than a generic version-mismatch.

It's contrived, but this code is well defined today but becomes racy under the proposed semantics:

😬 I don’t feel well with calling the proposed code “non-racy” just because it is not triggering the race-condition detection. I would in fact strongly assert that this is absolutely already racy code despite not being caught by the race detector.

That one can design a precarious piece of code that is both racy but does not trigger the race detector is indisputable, but one kind of has to expect that subtle changes can always easily “unexpectedly” break such precarious code.

@puellanivis, maybe you missed this detail, but the original is non-racy because the iteration only runs once (i < 1).

Increment is on a goroutine started by the original goroutine, so initialization happens-before increment. As those are the only two accesses, it's fine.

This change adds a third access at the end of the iteration, which races with the child goroutine.

@puellanivis, maybe you missed this detail, but the original is non-racy because the iteration only runs once (i < 1).

No, I did not miss that the code does not trigger the race detector because it’s secretly basically single-threaded. However, changing that 1 to a 2 and you now trigger the race detector, adding any access to x in the main body of the loop, or the 2nd or 3rd statements of the for loop, all also triggers the race detector.

This is specifically why I called the code “precarious”. It is inherently racey even though it does not trigger the race detector, which it does only because it has been specially designed to evade the race detector.

It doesn't trigger the race detector because there is no data race, not for a flaw in the race detector.
I think the distinction is important, because a read of your comments, to me, implies otherwise.

That the example is contrived is explicitly stated, and acknowledged in the reactions.
I similarly acknowledged that I failed find convincing examples of races introduced unwittingly by this change of behaviour.

PS: adding an access to x in the main body of the loop before starting the goroutine is not a data race either (starting a goroutine happens-before it running).

@puellanivis FWIW I agree with @ncruces and @mdempsky. The code is not racy. That it is easy to change it into racy code does not change that. Most race-free code can easily be modified into being racy.

It's not good code and it's certainly precarious code, which is why @mdempsky said it's contrived. But it currently has well-defined semantics and does not contain a race and it would contain a race under the proposed change.

However, when entertaining this change we are already accepting that it would break compatibility and trying to quantify that breakage. I hope we all agree that this example is sufficiently contrived not to measurably change "how breaking" changing loop-variables would be.

Not all loop captures are bugs. The vast majority of loop captures today are fine. The "compilation error" phase would break all of today's correct code, causing unnecessary churn.

That tool can be written and probably will be. Given how exceedingly rare the old behavior is the correct one, I am not 100% sure that using such a tool is the right way to approach a migration. The "git diff" is going to be error-prone and tedious. If you have good tests, testing and looking into failures is probably a better approach. But the tool will be important to have nonetheless.

To have a good implementation of such a tool it is important to have somewhat good escape analysis. You do not want all conversions to an interface or method calls to a pointer receiver to cause a "may escape" warning. If "(adjusted -m output)" happens like Russ mentioned, we have some preliminary evidence such a tool is likely to be reasonable for many people but not churn free though. (Take a look at the "Changing the semantics is usually a no-op" section for the evidence available.)

Therefore, another option might be for the "rewrite" tool to insert commented-out code

I had not yet considered creating a TODO list via comments yet. Such comments could searchable if the text is somewhat unique. It can be applied pre-transition. This would not be churn free, but it could be tackled over time, given as an onboarding project, additional tooling, etc. It may be a nice alternative to hosting the declaration before the loop, which could be forgotten (and is more likely to keep old bugs). Thanks for the idea.

A fair point, and yet another reason for the general rule that we don't make breaking changes. Generators would need to emit code that works with either semantics, but given the very low rate of code that is correct today and incorrect with the changed semantics, I suspect the vast majority of generators are fine already.

Thanks. I don't remember seeing that project before. I'm curious what analyzer it is using. I clicked on three issues at random from the first page of issues in rangeloop-pointer-findings, and only one of them is a real bug:

• theanalyst/rlyeh: main.go; 23 LoC github-vet/rangeloop-pointer-findings#18446
  real bug
• LLLSic/fuwujisuan: demo/entity/storage.go; 5 LoC github-vet/rangeloop-pointer-findings#18439
  not a bug, assuming filter doesn't keep &v
• google/knative-gcp: pkg/reconciler/intevents/pullsubscription/reconciler.go; 5 LoC github-vet/rangeloop-pointer-findings#18430
  not a bug, assuming IsControlledBy doesn't keep &dep

Yep! The goal of this project was to find all the instances with false positives and rely on crowdsourcing to filter through the false positives. That didn't pan out.

IIRC, the analyzer is based on looppointer with some additional customizations. It's been a while since I worked on it. The initial pass didn't use the type-checker. I started bringing in type-checking before being pulled away by a new job.

It is documented, the guidelines exist, the bugs are still introduced. We know for certain, based on empirical evidence, that this is not a solution.

Burying a change to the semantics of the language in go.mod is absolutely bonkers. It's supposed to be controlling modules, not the semantics of the language.

We already use go.mod to control the usage of other language features, like type parameters. That's exactly the intention behind the go directive in go.mod, which was added a long time ago.

this should just be a //go:newfor comment

This suggestion is addressed in the Go 2 transition document:

There is no obvious way to ever remove any of these special imports, so they will tend to accumulate over time. Python and Perl avoid the accumulation problem by intentionally making a backward incompatible change. After moving to Python 3 or Perl 6, the accumulated feature requests can be discarded. Since Go is trying to avoid a large backward incompatible change, there would be no clear way to ever remove these imports.

a prefor statement

If I understand you correctly, this is addressed in the "Changing loop syntax entirely would cause unnecessary churn" section of the top post.

If you have new data or new, so far not considered arguments, they would be welcome.

Per file controls is somewhat doable via build tags or a similar mechanism. That does not really solve what you are discussing though. My main concern about per file control is that this is not super compatible with the rest of the tooling and build ecosystem. (Kinda minor technical point but it does matter.) Per package controls would be quite a bit smoother to build today. That is a smaller unit that per module. But it is not as fine grained as per file or per loop.

FWIW there are discussions of having a tool that would annotate/rewrite existing loops that may escape. This could be done before or during an update. Doing this step would be opt-in. This would let folks that want to review over time do so. I realize this is not quite what you are requesting, but it may cover some concerns.

If the author knows to use special annotations, they at least probably already know how to avoid this. It's a better outcome to make it be less surprising for developers less familiar with Go.

what about making some small syntactical change to for loops at the same time, such that the old syntax doesn't compile and the new syntax works after changing go.mod?

Please see the section "Changing loop syntax entirely would cause unnecessary churn" in the top post.

From the top post:

In the 3-clause form, the start of the iteration body copies the per-loop i into a per-iteration i, and then the end of the body (or any continue statement) copies the current value of the per-iteration i back to the per-loop i. Unless a variable is captured like in the above example, nothing changes about how the loop executes.

The loop variables are copied back-and-forth so nothing changes. You can still modify i within the 3-clause loop and the modification will be copied to the variable for the next iteration.

Oh, I've missed that.
In that case, I see no problems with the proposition.

Thanks for clarifying. I do disagree with the how the arguments associated with that branch of the dichotomy apply to the example of for i,v,p := range slice to conclude 'more churn'. Even more so when considering
the net churn of combination of for I,v,p := range slice applied first with opt-in application to clean up &v then per-iteration semantics.

W.r.t. undo proliferation of ways to express loops causing confusion, I do not feel that for i,v,p := range slice or something like it would be undo proliferation. Certainly, the proliferation of functional+generic looping constructs mixed with the current loop syntax seems worse w.r.t. this criterion.

Certainly, the proliferation of functional+generic looping constructs mixed with the current loop syntax seems worse w.r.t. this criterion.

I don't know what you mean by "functional". The generic iterator design does indeed add a new looping construct and thus does add the same level of overhead, yes. And it has to pay for that cost by demonstrating a commensurate benefit. Note that I also pointed out that your suggestion does not actually help solving the problem, as it still requires a programmer to know to use it - and if they know, they can already use x := x. So your suggestion has very little benefit.

Note that I also pointed out that your suggestion does not actually help solving the problem, as it still requires a programmer to know to use it - and if they know, they can already use x := x. So your suggestion has very little benefit.

It would help me solve the problem, as outlined previously, by providing a convenient construct which encourages avoiding &v within for _, v := range items and can be used as a candidate replacement for &v where that may appear in code, all without anywhere near the backward incompatibility impact of changing semantics. &v was one of the items cited in the code analysis.

I disagree that the problem is requiring an understanding of allocations and scope -- I don't get that impression from the top doc either. Personally, I like that Go encourages understanding of scope and would discourage x := x for what I find to be obvious readability deficiencies. If I had to characterise the relationship between knowledge requirement and the stated problem, I'd say that Go as it is today fails to make understanding the scope+allocations around loop variables clear in the case of &v and closures. Similarly, I think Go is a bit too verbose for capture by value in nested closures.

Both of these problems can be alleviated without changing semantics. Alleviating these problems would in turn reduce the backward incompatible impact of changing the loop semantics, should that occur.

It would help me solve the problem, as outlined previously, by providing a convenient construct which encourages avoiding &v within for _, v := range items

You can already do that with &items[i]. So, again: If you know there's a problem, you can already solve it. The problem is that people still make those mistakes. That's the problem we are trying to fix.

@scott-cotton "or incur calculating a slice of pointers." was what I was alluding to. This is obviously not always appropriate, and &item[i] is available for the general case. My main point in suggesting a function that constructs a slice of pointers is that the specific example in the top post has an alternative appealing library solution. While the "Item&" issue is interesting, I hope the conversation will not get too hung up on this specific example.

Personally I actually worry a lot less about the "&v" cases. Those make you write an "&" to give you a hint that an address was just taken and to think about storage duration. The hard to locally reason about cases are closures, interface capture, and method receivers. Those just use v and are influenced by other functions and maybe even functions in different packages.

FWIW an unfortunate characteristic of have some range statement that produces pointers like for index, item, ptr := range items is that items must be a slice, an array or a ptr to an array. It cannot be a map, a channel or a string.

Do you have a real program which depends on the current semantics? That is the question of this thread. Your original answer was "a program which relies on the spec guaranteeing a copy not happening would", which is simply wrong. The spec makes no such guarantee.

You can discuss that if you find it interesting, but i suggest doing it on a different forum.

To me, this implies that variables are not re-declared in each iteration, because the spec refers to assignments (which are '=' not ':=') and continues the pattern of distinguishing declaration and assignment/use in describing what happens on each iteration.

You seem to be talking now about the number of declarations, whereas your original statement was about the number of copies. I think that is a source of confusion here. The spec currently guarantees that a program executes as if there is one copy from a ranged collection to the second range variable per execution of the loop, and that guarantee will not change with the proposal. The spec guarantees that a range loop declares its variables exactly once, which is exactly the thing proposed to be changed. So, your argument now reads as "a program that currently depends on there being one declaration per loop would be an example of a program that currently depends on there being one declaration per loop."