The most interesting suggestion in the comments is that the compiler should flag potentially breaking changes with // TODO: review loop closure when you run go mod edit -go=affectedversion
I’m also in favour of a further change - making the loop variable alias when using a range loop, so that the underlying array element can be mutated.
I know one can use the index variable for this. However:
at the moment, any mutating method will have its effects silently discarded (it mutates the temporary copy)
I’m not aware of a downside to this (it would be great if someone could suggest some)
Motivating use case:
you have a method on an object which returns some expensive-to-compute property
you loop over a collection of these objects calling this method and using the value, perhaps more than once
later, you modify the property-getting function to update and use a private cache of the property in the object to avoid re-computation
result: the cache is not used, since the internal cache is only updated in the loop variable copy, never the element of the underlying array.
Again - this is all fixable when you notice it, but the “silently discard the mutated copy” behaviour is poor (and has bitten me in the past). The problem manifests as “my performance improvement didn’t work”.
The GitHub thread at its core is about scoping language change so that users would agree for it to arrive quickly via a minor version (1.x), despite the fact the change is mildly breaking.
Your own proposal is very much breaking, because there is a ton of code that modifies the range-iterated var today (when there is any computation within a loop it is a convenient local variable). So it would need either a different keyword than “range”, or it would be a v2 discussion.
So what would be the type of the loop variable — T, or *T?
T - we’re aliasing the underlying elt.
And you’re right. Explicit invocation of a function with an arg of type T would cause a copy (as it does outside a loop).
However, for methods golang chooses to implicitly call methods with a *T receiver on values of type T. So:
for _, v := range map[int]T{...} {
// set all 'elem' to "abc", since we are aliasing. Same as outside loop.
v.elem = "abc"
// Same as f(v) outside loop, no effect (due to copy in the func arg, not loop copy)
f(v) // func f(v T) { v.elem = "def" }
// Same as g(v) outside loop, mutate v
g(v) // func g(v *T) { v.elem = "ghi" }
// Same as v.h() outside loop, no effect (due to copy in the receiver, not loop copy)
v.h() // func (v T) h() { v.elem = "jkl" }
// Same as v.i() outside loop, mutate v
v.i() // func (v *T) i() { v.elem = "mno" }
}
or am I missing something due to the map in your example? I’m sorry - I don’t see why that is different from a loop over a slice (since we are only considering the map values, not keys).
The model I am thinking of is “on each iteration of the loop for i, v := range xs the value v is equivalent to xs[i]. i.e. using i is only useful for cases where you want the numeric value of the index, not to mutate the underlying array.
Map values aren’t addressable in Go, if you want to call a pointer-receiver method on one you have to copy the value out, call the method, and assign the result back. There are reasons for that, and I don’t think you can get away with breaking/sidestepping that rule.
am I missing something due to the map in your example? I’m sorry - I don’t see why that is different from a loop over a slice (since we are only considering the map values, not keys).
No, map or slice, shouldn’t matter.
on each iteration of the loop for i, v := range xs the value v is equivalent to xs[i]
So the original motivation (or at least one of them) for the current behavior was so that each range clause would only need to allocate a single value for the second parameter, and re-use it for each iteration. I’m not positive, but I believe this approach would require a new allocation per iteration.
I prefer this behavior too / find it intuitive. It’s what I go with in my little Go->C++ transpiler – https://github.com/nikki93/gx. The generated C++ does for (auto &elem : collection).
The most interesting suggestion in the comments is that the compiler should flag potentially breaking changes with
// TODO: review loop closure
when you rungo mod edit -go=affectedversion
Very much in favour of this.
I’m also in favour of a further change - making the loop variable alias when using a range loop, so that the underlying array element can be mutated.
I know one can use the index variable for this. However:
Motivating use case:
Again - this is all fixable when you notice it, but the “silently discard the mutated copy” behaviour is poor (and has bitten me in the past). The problem manifests as “my performance improvement didn’t work”.
The GitHub thread at its core is about scoping language change so that users would agree for it to arrive quickly via a minor version (1.x), despite the fact the change is mildly breaking.
Your own proposal is very much breaking, because there is a ton of code that modifies the range-iterated var today (when there is any computation within a loop it is a convenient local variable). So it would need either a different keyword than “range”, or it would be a v2 discussion.
So what would be the type of the loop variable —
T
, or*T
?If the loop variable were a struct type, what would happen in each of these bodies?
That’s a good point. I think my answers are:
T - we’re aliasing the underlying elt.
And you’re right. Explicit invocation of a function with an arg of type T would cause a copy (as it does outside a loop).
However, for methods golang chooses to implicitly call methods with a
*T
receiver on values of typeT
. So:or am I missing something due to the map in your example? I’m sorry - I don’t see why that is different from a loop over a slice (since we are only considering the map values, not keys).
The model I am thinking of is “on each iteration of the loop
for i, v := range xs
the valuev
is equivalent toxs[i]
. i.e. usingi
is only useful for cases where you want the numeric value of the index, not to mutate the underlying array.Map values aren’t addressable in Go, if you want to call a pointer-receiver method on one you have to copy the value out, call the method, and assign the result back. There are reasons for that, and I don’t think you can get away with breaking/sidestepping that rule.
No, map or slice, shouldn’t matter.
So the original motivation (or at least one of them) for the current behavior was so that each range clause would only need to allocate a single value for the second parameter, and re-use it for each iteration. I’m not positive, but I believe this approach would require a new allocation per iteration.
I prefer this behavior too / find it intuitive. It’s what I go with in my little Go->C++ transpiler – https://github.com/nikki93/gx. The generated C++ does
for (auto &elem : collection)
.