5 ms·

Fixing the other Go loop bug

62 points by dmarto 3y ago

fifilura 3y ago
This is typical of the bugs you have to worry about when working with a language that favors mutability. With immutables you simply map on players list and continue working with the copy. Bugs avoided. Performance concerns - maybe they exist - but show me them first.
- ptsneves 3y ago
  Incredibly insightful comment, even the performance concern. Apologies for doing a glorified +1 comment, but if you take the above comment plus the article's in-loop mutation algorithm and try it in different languages you get insights on who you are as a coder and what are the language's strengths. Another detail is that the programmer in the article prefers an mutation iterator format while the language forces him/her to use an array/index. This has further impact on pointer vs value accesses. Also by mapping on players list the above post means: just discard the previous one, create a new one however you want and make the new one the new source of truth. No need for mutability considerations.
  - Joker_vD 3y ago
    > just discard the previous one, create a new one however you want I want to create it by re-using the memory of the previous one that I've just discarded, please (well, the one that will end up discarded, actually: it has to live until the map ends executing, hasn't it). Unfortunately, most of the languages' implementations/runtimes don't employ the Perceus algorithm.
    - fifilura 3y ago
      Can you describe why you need it? Is it because of real performance concerns or just because it feels better knowing that you don't waste memory space and CPU cycles? If you really need it, you can always fall back to using mutables for this special list, and use immutables for the rest? This is probably the reason why languages don't implement the Perceus algorithm. If you really needed it you might as well do it by hand.
    - duped 3y ago
      In a managed language that's the job of the compiler. You don't need a fancy reference counter either, peephole optimizations will do.
- aatd86 3y ago
  Perhaps, perhaps not. I prefer mutability. The real thing here is the definition of the Player type. It should have a mutating method that resets it's score (a method defined on *Player). Raw access on fields is often only ok for naive, trivial implementations. Methods are often needed anyway. That's straightforward. Immutability doesn't make any sense here.
  - fifilura 3y ago
    > I prefer mutability ... Examples on how to avoid the bug ... Notable is that you claim here is an opinion not an argument. That immutability is not needed is also not an argument against it. The point here is that working with immutables relieves you from some cognitive stress and repetitive work such as writing accessors and making fields private.
    - aatd86 3y ago
      It's not just an opinion. With mutability, it's still possible to make things immutable if properly encapsulated. If everything is immutable by default and you need mutability for some reason, good luck. The heuristic is simple too. If you need something to be mutable, you should deal with pointers. So *Player and []*Player here.
      - shirogane86x 3y ago
        I always felt like it was kind of the other way around? Speaking from experience , most languages where immutability is the default and/or the only option can usually simulate mutability or have it as a stdlib-provided escape hatch (elixir and haskell come to mind), but most languages that are mutability-first really struggle to provide any sort of immutability that is reliable.
        aatd86 3y ago
        At a datastructure level, perhaps, although functional datastructures are pretty fringe. (cf. Okasaki) In general, they can still be implemented by traditional imperative languages (obviously). At the type/variable level, it's the opposite. We see it with const. Once it's in here, it's in there.
tylerhou 3y ago
This is not really a bug -- it's a natural consequence of supporting value semantics. The Go code var player Player otherPlayer := player otherPlayer.score = 0 has the exact same "bug." This isn't something that should be fixed by the language. Instead, a linter rule that detects that the write is never read would catch this.
- tapirl 3y ago
  The author didn't say it is a bug. The author just says this is a shortcoming of Go.
  - coder543 3y ago
    > The author didn't say it is a bug. The author's title literally refers to it as a bug.
  - Joker_vD 3y ago
    For reasons it’s never really bothered me, but the *other* loop bug does bite me some times. This is the one where the loop values are values. The common mistakes wiki lists the capture bug twice, but doesn’t mention this bug. Yeah, I guess you're right, the author didn't call it a bug...
    - tapirl 3y ago
      I believe the author doesn't mean it is a bug in the general sense. The author just means it is a shortcoming, and this shortcoming is surely fixable. The author provides a solution, but I think the zig way is better: for _, *player := range players { player.score = 0 }
      - lsaferite 3y ago
        If they meant 'shortcoming', they would have used that word. They used the word 'bug' to describe the issue. Repeatedly.
- 3y ago
rurban 3y ago
Not a bug but a feature. Go favors concurrency safety and concurrency efficiency. Therefore it has to copy loop values to be able to parallelize them. With a reference this would be unsafe to do.
- Joker_vD 3y ago
  What are you talking about? Go doesn't parallelize its loops, in fact, concurrent mutating iteration is unsafe. Reads of memory locations larger than a single machine word are encouraged but not required to meet the same semantics as word-sized memory locations, observing a single allowed write w. For performance reasons, implementations may instead treat larger operations as a set of individual machine-word-sized operations in an unspecified order. This means that races on multiword data structures can lead to inconsistent values not corresponding to a single write. When the values depend on the consistency of internal (pointer, length) or (pointer, type) pairs, as can be the case for interface values, maps, slices, and strings in most Go implementations, such races can in turn lead to arbitrary memory corruption. The for-range in Go doesn't introduce any synchronization points.
  - rurban 3y ago
    You cited something else. They don't yet parallelize loops, but the idea was there. Most others do it already
    - Joker_vD 3y ago
      If you mean OMP-style for loops, they most likely won't add them: [0]. [0] https://github.com/golang/go/issues/24210#issuecomment-369793380 https://github.com/golang/go/issues/24210#issuecomment-36979...

62 points by dmarto 3y ago

This is typical of the bugs you have to worry about when working with a language that favors mutability. With immutables you simply map on players list and continue working with the copy. Bugs avoided. Performance concerns - maybe they exist - but show me them first.

ptsneves 3y ago
Incredibly insightful comment, even the performance concern. Apologies for doing a glorified +1 comment, but if you take the above comment plus the article's in-loop mutation algorithm and try it in different languages you get insights on who you are as a coder and what are the language's strengths. Another detail is that the programmer in the article prefers an mutation iterator format while the language forces him/her to use an array/index. This has further impact on pointer vs value accesses. Also by mapping on players list the above post means: just discard the previous one, create a new one however you want and make the new one the new source of truth. No need for mutability considerations.
- Joker_vD 3y ago
  > just discard the previous one, create a new one however you want I want to create it by re-using the memory of the previous one that I've just discarded, please (well, the one that will end up discarded, actually: it has to live until the map ends executing, hasn't it). Unfortunately, most of the languages' implementations/runtimes don't employ the Perceus algorithm.
  - fifilura 3y ago
    Can you describe why you need it? Is it because of real performance concerns or just because it feels better knowing that you don't waste memory space and CPU cycles? If you really need it, you can always fall back to using mutables for this special list, and use immutables for the rest? This is probably the reason why languages don't implement the Perceus algorithm. If you really needed it you might as well do it by hand.
  - duped 3y ago
    In a managed language that's the job of the compiler. You don't need a fancy reference counter either, peephole optimizations will do.
aatd86 3y ago
Perhaps, perhaps not. I prefer mutability. The real thing here is the definition of the Player type. It should have a mutating method that resets it's score (a method defined on *Player). Raw access on fields is often only ok for naive, trivial implementations. Methods are often needed anyway. That's straightforward. Immutability doesn't make any sense here.
- fifilura 3y ago
  > I prefer mutability ... Examples on how to avoid the bug ... Notable is that you claim here is an opinion not an argument. That immutability is not needed is also not an argument against it. The point here is that working with immutables relieves you from some cognitive stress and repetitive work such as writing accessors and making fields private.
  - aatd86 3y ago
    It's not just an opinion. With mutability, it's still possible to make things immutable if properly encapsulated. If everything is immutable by default and you need mutability for some reason, good luck. The heuristic is simple too. If you need something to be mutable, you should deal with pointers. So *Player and []*Player here.
    - shirogane86x 3y ago
      I always felt like it was kind of the other way around? Speaking from experience , most languages where immutability is the default and/or the only option can usually simulate mutability or have it as a stdlib-provided escape hatch (elixir and haskell come to mind), but most languages that are mutability-first really struggle to provide any sort of immutability that is reliable.
      - aatd86 3y ago
        At a datastructure level, perhaps, although functional datastructures are pretty fringe. (cf. Okasaki) In general, they can still be implemented by traditional imperative languages (obviously). At the type/variable level, it's the opposite. We see it with const. Once it's in here, it's in there.

tylerhou 3y ago

This is not really a bug -- it's a natural consequence of supporting value semantics. The Go code var player Player otherPlayer := player otherPlayer.score = 0 has the exact same "bug." This isn't something that should be fixed by the language. Instead, a linter rule that detects that the write is never read would catch this.

tapirl 3y ago
The author didn't say it is a bug. The author just says this is a shortcoming of Go.
- coder543 3y ago
  > The author didn't say it is a bug. The author's title literally refers to it as a bug.
- Joker_vD 3y ago
  For reasons it’s never really bothered me, but the *other* loop bug does bite me some times. This is the one where the loop values are values. The common mistakes wiki lists the capture bug twice, but doesn’t mention this bug. Yeah, I guess you're right, the author didn't call it a bug...
  - tapirl 3y ago
    I believe the author doesn't mean it is a bug in the general sense. The author just means it is a shortcoming, and this shortcoming is surely fixable. The author provides a solution, but I think the zig way is better: for _, *player := range players { player.score = 0 }
    - lsaferite 3y ago
      If they meant 'shortcoming', they would have used that word. They used the word 'bug' to describe the issue. Repeatedly.
3y ago

rurban 3y ago

Not a bug but a feature. Go favors concurrency safety and concurrency efficiency. Therefore it has to copy loop values to be able to parallelize them. With a reference this would be unsafe to do.

Joker_vD 3y ago
What are you talking about? Go doesn't parallelize its loops, in fact, concurrent mutating iteration is unsafe. Reads of memory locations larger than a single machine word are encouraged but not required to meet the same semantics as word-sized memory locations, observing a single allowed write w. For performance reasons, implementations may instead treat larger operations as a set of individual machine-word-sized operations in an unspecified order. This means that races on multiword data structures can lead to inconsistent values not corresponding to a single write. When the values depend on the consistency of internal (pointer, length) or (pointer, type) pairs, as can be the case for interface values, maps, slices, and strings in most Go implementations, such races can in turn lead to arbitrary memory corruption. The for-range in Go doesn't introduce any synchronization points.
- rurban 3y ago
  You cited something else. They don't yet parallelize loops, but the idea was there. Most others do it already
  - Joker_vD 3y ago
    If you mean OMP-style for loops, they most likely won't add them: [0]. [0] https://github.com/golang/go/issues/24210#issuecomment-369793380 https://github.com/golang/go/issues/24210#issuecomment-36979...