Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: slices: add AllPointer #69345

Closed
myaaaaaaaaa opened this issue Sep 8, 2024 · 9 comments
Closed

proposal: slices: add AllPointer #69345

myaaaaaaaaa opened this issue Sep 8, 2024 · 9 comments
Labels
Milestone

Comments

@myaaaaaaaaa
Copy link

myaaaaaaaaa commented Sep 8, 2024

Proposal Details

Currently, ranging over slices and arrays only provides read access since the elements are copied by value. Adding a way to modify them when using range would enable loops to omit using index variables entirely for most cases, apart from the rare cases when index arithmetic is actually necessary.

This would, among other things, prevent accidentally using the wrong index when writing nested loops.

func ExampleRange() {
	nums := make([]int, 5)

	for _, n := range nums { // or slices.All(nums) - same issue occurs
		n = 1 // n is a copy so this doesn't modify the slice
	}
	fmt.Println(nums)
	//Output:
	//[0 0 0 0 0]


	for _, n := range slices.AllPointer(nums) {
		*n = 2 // Modifies the slice
	}
	fmt.Println(nums)
	//Output:
	//[2 2 2 2 2]


	var numsArray [5]int
	for _, n := range slices.AllPointer(numsArray[:]) {
		*n = 3 // Works with arrays too
	}
	fmt.Println(numsArray)
	//Output:
	//[3 3 3 3 3]
}

Implementation

package slices

func AllPointer[S ~[]E, E any](s S) iter.Seq2[int, *E] {
	return func(yield func(int, *E) bool) {
		for i := range s {
			if !yield(i, &s[i]) {
				return
			}
		}
	}
}
@gopherbot gopherbot added this to the Proposal milestone Sep 8, 2024
@gabyhelp
Copy link

gabyhelp commented Sep 8, 2024

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@zigo101
Copy link

zigo101 commented Sep 8, 2024

Pointers might be a better name.

@MikeMitchellWebDev
Copy link
Contributor

MikeMitchellWebDev commented Sep 8, 2024

I don't think using "the wrong index when writing nested loops" is much of a problem. Looping with an index variable is such a common activity in every programming language.

@zigo101
Copy link

zigo101 commented Sep 9, 2024

Agree that "the wrong index when writing nested loops" should not be the main reason for this proposal.
Other than that, this proposal is the most creative use of iterators I have seen up to now.

@earthboundkid
Copy link
Contributor

Writing p := &a[i] as the first line of a loop is inelegant, but everyone knows what it means without looking it up, and it doesn't require any imports. As a code reader, I'm not infrequently annoyed that someone wrote a[i] five times instead of p := &a[i] once and then using p thereafter, but in a world where slices.AllPointer exists, I would just be annoyed that they didn't use slices.AllPointer instead. It doesn't really solve the core problem, which is developers not thinking to do the simplification.

@myaaaaaaaaa
Copy link
Author

myaaaaaaaaa commented Sep 11, 2024

In an ideal world, slices.AllPointer() would be built into the language, but the cleanest way I could find to retrofit that into Go hit an unfortunate backwards compatibility snag (see #69337 (comment)).

Barring that, it becomes a question of choosing between the remaining mediocre options. Emoji voting isn't in favor of this particular one, so I'll close this now.

For future reference, another mediocre option that already exists in the language is to shadow the index variable:

	for p := range s {
		p := &s[p]

		// ...
	}

@myaaaaaaaaa myaaaaaaaaa closed this as not planned Won't fix, can't repro, duplicate, stale Sep 11, 2024
@mitar
Copy link
Contributor

mitar commented Sep 14, 2024

Oh, I would like this as well (or something like #69337 or #21537). I thought that changes in for loop range iteration in 1.22 will make this easier to add, but it seems from #69337 Golang maintainers are still not for it? It is really one of very common pain points (even if the solution is easy) . I think what @griesemer wrote here still stands, the fact that this asymmetry exists is bothersome. We got rid of one reason to write x := x, let's maybe get rid also of x := &s[i].

My main motivation for this is that I would like to be changing elements in a slice while I am iterating over the slice. Currently I have to reassign at the end of the loop to the slice (like s[i] = x).

@mateusz834
Copy link
Member

mateusz834 commented Sep 14, 2024

Golang maintainers are still not for it?

This issue was closed by the author, it was not declined by the proposal team.

@mitar
Copy link
Contributor

mitar commented Sep 14, 2024

This issue was closed by the author, it was not declined by the proposal team.

For closing by the team I meant #69337. This is why I am writing here to the author to maybe reopen this, if this might be more to their liking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants