10 ms·
Proposal: Less error-prone loop variable scoping
- benhoyt 3y agoDefinitely a good change, as this fixes an issue I (and I think most Go developers) have been bitten by more than once. It was really interesting to read the response of someone from the C# team in the original discussion, of how C# had gone through almost exactly the same change and how that went down: https://github.com/golang/go/discussions/56010#discussioncomment-3788526 https://github.com/golang/go/discussions/56010#discussioncom...
- jrochkind1 3y agoIt is so useful to compare analogous experiences from other languages, and I find not done often enough. For instance, i feel like ruby and python could probably learn a lot from each other, but it can seem like there just aren't enough people (myself also not) who are sufficiently expert at both to identify the lessons.
- tapirl 3y agoNot exactly. C# had never gone through the change on c-like for-loops.
- kreetx 3y agoYup, not a Go developer but ran into exactly this years ago when wanted to get a feel of it. Took a while to figure out what was wrong!
- doctor_eval 3y agoAhh - I hit this exact problem just last week. I was taking the address of the loop variable ranging over a slice, and was surprised when my resulting slice was full of pointers to the same thing. I wasn’t aware of the v:= v idiom. Fortunately it was pretty obvious what was going on, but certainly the behaviour wasn’t what I expected while I was “in the flow”. Super happy to see these little cases getting fixed!
- tapirl 3y agoThe proposal has two parts, one is to change the semantic of "for-range" loops, the other is to change the semantic of traditional c-like "for;;" loops. The first part is good and will fix your problem. However the second part will produce some surprises to many people.
- doctor_eval 3y agoI hadn't seen that, but I've since read the comments from rsc and others and I'm curious why you think it will be a problem? From what I can gather (I'm too busy/lazy/both to dive into the formalities), the for;; loop will have a new variable allocated at the start of each iteration, similar to the proposed range loop behaviour change. The strong argument for this is that you'd expect both loop forms to behave similarly; if they were to remain different, which would be the case of they didn't make this change, then the principle of least surprise would be violated. However, reading the followup comments, some people seem to have missed that the value of the variable at the end of one iteration will be used to re-initialise the new variable at the start of the next iteration. This seems to mean that the usual semantics of a for;; loop will not change, other than if you're doing some really strange stuff, which doesn't seem to be the case when reviewing large existing code bases. I had misgivings about the behavioural changes being triggered by the Go version number in go.mod, but rsc makes a good argument that any dependency change carries some risk, intentional or not.
- tapirl 3y agosee https://news.ycombinator.com/item?id=35912510 https://news.ycombinator.com/item?id=35912510
- doctor_eval 3y agoYeah - but I agree with rsc, it’s easy to come up with constructed examples of behaviour changes, much harder to come up with examples you’d find in production code. I mean - maybe you feel differently, but if I saw that code in a PR, I would reject it. It’s way too tricky for me.
- amluto 3y agoI have to admit that, as a C/C++ programmer, I find this entire thing a bit bizarre. In C and C++, of course a for loop induction variable is reused for each iteration. And if you take the address of the variable and keep it around for too long, you get to keep all the pieces and possibly the exploitable UAF vulnerability, too. (Never mind that, prior to C99, C induction variables were always in an enclosing scope, and this would be fairly obvious from reading the code.) In Rust, code like the examples wouldn’t compile. If you want to copy a value to the heap and take a reference, you need to say so. And it will be quite clear whether you are keeping a boxed copy or whether you are keeping a reference to the original object. (And the object can’t mutate out from under you unexpectedly in either case — a non-mutable reference prevents mutation!) So the fact that, in Go, one might rely on automatic promotion of an induction variable (which obviously gets mutated every iteration!) to the heap, and thus get confused about precisely which value is promoted to the heap, seems weird from my perspective. IMO one might reasonably argue that the problem is that this pattern works at all, not that it works in the way that people usually don’t want. edit: in C++, with range-based for, the induction variable is gone after each iteration. So taking a reference that outlives the iteration is invalid, and if you’re lucky, the compiler will warn. The confusing cases from the Go proposal are that invalid in C++, not confusing.
- rcme 3y agoHow often do you access your loop variables asynchronously?
- amluto 3y agoRarely, if ever. But aliasing and unexpected mutability don’t need asynchrony or threading to be a problem. For example, the entire problem leading to this proposal is about unexpected mutability. You have a variable i, which presently has a specific value. You take a reference to it, and you give that reference to some other code that saves it. Then you mutate i, and you are suddenly surprised that the external code now has a reference to the new value.
- rcme 3y ago
- philosopher1234 3y agoThe slightly less obvious story here: this is a breaking change to Go, and they’ve managed to find a policy whereby such a change is permissible. I think that’s the more substantial development, though this is quite an ugly wart and I’m glad it’s off.
- GauntletWizard 3y agoYeah, I like this change, but I would like this change for Golang 2.0.
- pansa2 3y agoAgreed. The change (especially to for-range) is a good one, but IMO they shouldn’t be breaking the Go 1.x Compatibility Promise in a Go 1.x release.
- arp242 3y ago"Go 2" will most likely never happen. It was always a placeholder for "yeah, this might be a good idea, but we can't do it now as it would break compatibility".
- legobmw99 3y agoThis doesn’t sound that different from e.g. Rust’s Editions, though it seems like it can occur much more frequently
- philosopher1234 3y agoI dont really know much about Rust editions, but what little I do know sounds at least similar. Heres the issue where they changed the backcompat policy: https://github.com/golang/go/issues/56986 https://github.com/golang/go/issues/56986
- j16sdiz 3y agoI would rather not have this. The worse thing is: the old code still compile in new /old version. There is no error or warning. Updating the version is so innocent looking. Nobody will be able to catch this error.
- tapirl 3y agoMany people think the change is only applied to "for-range" loops. However, in fact it will be also applied to traditional c-like "for ;;" loops, which at least will produce two surprises to old Go programmers: https://github.com/golang/go/issues/60078#issuecomment-1544324607 https://github.com/golang/go/issues/60078#issuecomment-15443...
- xyzzy_plugh 3y agoWhat surprise is that? It would appear that no one will be surprised because the scenario necessary is never actually used anywhere. It doesn't exist. For a change to be breaking it needs the potential to be able to break. As the potential approaches zero, so too does the coefficient of breakage.
- tapirl 3y agoOne surprise is that the following loop will print false (now it prints true). for i, p := 0, (*int)(nil); p == nil; print(p == &i) { p = &i } Another surprise is that the value a in the following loop will be duplicated twice in each loop step (now it never gets duplicated). func foo(constFunc func(*[100000]int)) { for a, i := [100000]int{}, 0; i < len(a); i++ { constFunc(&a) } }
- arp242 3y agoThat first example seems rather artificial; I can't recall I've ever had to do anything like that. What are real-world use cases? That second example seems like a rather convoluted way of looping 100000 times.
- Patrickmi 3y agoSometimes I encounter this like 1/10 but it’s annoying for those who hasn’t seen it and not using tests, it’s really good seeing focus on this just hope how they’ll resolve it with Go version
- kgeist 3y agoI wish Go also had this: for v := range vs {} instead of this: for _, v := range vs {} How often do you need to have both the element and its index? I'd say it's not the majority of cases. And now you have to type "_," in 99% loops. That would be a far more serious breaking change, however. Although, that would only in practice affect integer lists (i.e. code silently compiles). For other types, the compiler would complain about type mismatch errors and refuse to compile (which is better than nothing).
- tapirl 3y agoI have the same feeling as you. Go indeed has for v := range vs {} But unfortunately here v means element index.
- masklinn 3y ago> How often do you need to have both the element and its index? I'd say it's not the majority of cases. And now you have to type "_," in 99% loops. Needing either the index or both is likely a lot more common in Go than in other languages owing to the lack of any sort of iterator or composition. So e.g. any sort of zipping or in-place mapping requires the index. > Although, that would only in practice affect integer lists (i.e. code silently compiles). Not so. It would also affect strings (runes are integers), as well as maps with the same key and value types. Plus it could affect every other case depending how the item is used.
- kgeist 3y ago>Needing either the index or both is likely a lot more common in Go than in other languages owing to the lack of any sort of iterator or composition In our codebase, there's very few instances of needing the index. Usually you just want to iterate over a list and apply some function(s) to the elements or add them to another list.