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

slices: Unclear documentations #60142

Open
merykitty opened this issue May 12, 2023 · 1 comment
Open

slices: Unclear documentations #60142

merykitty opened this issue May 12, 2023 · 1 comment
Labels
Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@merykitty
Copy link

Dissimilar to the corresponding data structures in other languages such as vector in C++ or ArrayList in Java, a Go slice does not own the data it contains but is a mere mutable view over a backing array. As a result, slices can share a common backing array and changes made to the data through one can be observable by the others. This requires operations mutating the data of a slice expressing their behaviour clearly and covering all cases, which seems to be absent from the slices package.

Suggestions:

  // Insert inserts the values v... into s at index i,
  // returning the modified slice.
  // The elements at s[i:] are shifted up to make room.
  // In the returned slice r, r[i] == v[0],
  // and r[i+len(v)] == value originally at r[i].
+ // Insert operates as if it reads all data from s and v before doing
+ // any modification.
+ // Insert creates a new backing array and leaves
+ // the existing ones unchanged if len(s) + len(v) > cap(s)
+ // Otherwise, it modifies the backing array of s and the
+ // returned slice has the same backing array as s
  // Insert panics if i is out of range.
  // This function is O(len(s) + len(v)).
  func Insert[S ~[]E, E any](s S, i int, v ...E) S

  // Delete removes the elements s[i:j] from s, returning the modified slice.
  // Delete panics if s[i:j] is not a valid slice of s.
- // Delete modifies the contents of the slice s; it does not create a new slice.
+ // Delete modifies the contents of the backing array of s, it does not create
+ // a new backing array, the returned slice has the same backing array as s.
+ // Delete operates as if it reads all data from s before doing any modification.
  // Delete is O(len(s)-j), so if many items must be deleted, it is better to
  // make a single call deleting them all together than to delete one at a time.
- // Delete might not modify the elements s[len(s)-(j-i):len(s)]. If those
+ // Delete does not modify the elements s[len(s)-(j-i):len(s)]. If those 
  // elements contain pointers you might consider zeroing those elements so that
  // objects they reference can be garbage collected.
  func Delete[S ~[]E, E any](s S, i, j int) S

  // DeleteFunc removes any elements from s for which del returns true,
  // returning the modified slice.
- // DeleteFunc modifies the contents of the slice s;
- // it does not create a new slice.
- // When DeleteFunc removes m elements, it might not modify the elements
+ // DeleteFunc operates as if it reads all data from s before doing any modification
+ // DeleteFunc modifies the contents of the backing array of s, it does not create
+ // a new backing array, the returned slice has the same backing array as s.
+ // When DeleteFunc removes m elements, it does not modify the elements
  // s[len(s)-m:len(s)]. If those elements contain pointers you might consider
  // zeroing those elements so that objects they reference can be garbage
  // collected.
  func DeleteFunc[S ~[]E, E any](s S, del func(E) bool) S

  // Replace replaces the elements s[i:j] by the given v, and returns the
- // modified slice. Replace panics if s[i:j] is not a valid slice of s.
+ // Replace operates as if it reads all data from s and v before doing
+ // any modification.
+ // Replace creates a new backing array and leaves the existing ones
+ // unchanged if len(s) - (j-i) + len(v) > cap(s),
+ // Otherwise, it modifies the backing array of s and the
+ // returned slice has the same backing array as s.
+ // Replace panics if s[i:j] is not a valid slice of s.
  func Replace[S ~[]E, E any](s S, i, j int, v ...E) S

- // Clone returns a copy of the slice.
+ // Clone returns a new slice with the same elements as those of s
+ // with a newly created backing array.
  // The elements are copied using assignment, so this is a shallow clone.
  func Clone[S ~[]E, E any](s S) S

  // Compact replaces consecutive runs of equal elements with a single copy.
  // This is like the uniq command found on Unix.
- // Compact modifies the contents of the slice s; it does not create a new slice.
- // When Compact discards m elements in total, it might not modify the elements
+ // Compact operates as if it reads all data from s and v before doing
+ // any modification.
+ // Compact modifies the contents of the backing array of s, it does not create a
+ // new backing array, the returned slice has the same backing array as s.
+ // When Compact discards m elements in total, it does not modify the elements
  // s[len(s)-m:len(s)]. If those elements contain pointers you might consider
  // zeroing those elements so that objects they reference can be garbage collected.
  func Compact[S ~[]E, E comparable](s S) S

  // Grow increases the slice's capacity, if necessary, to guarantee space for
  // another n elements. After Grow(n), at least n elements can be appended
- // to the slice without another allocation. If n is negative or too large to
- // allocate the memory, Grow panics.
+ // Grows creates a new backing array if len(s) + n > cap(s), in which case
+ // the returned slice has a different backing array from s.
+ // Otherwise, Grows returns s unchanged.
+ // If n is negative or too large to allocate the memory, Grow panics.
 func Grow[S ~[]E, E any](s S, n int) S

  // Clip removes unused capacity from the slice, returning s[:len(s):len(s)].
+ // Clip does not modify the backing array of s.
  func Clip[S ~[]E, E any](s S) S
@randall77
Copy link
Contributor

This feels to me kind of an esoteric corner case. As such, it would be nice if we could document it once somewhere and not have to repeat it for each function in question.

Kind of like how the sort package describes issues with making Less a transitive ordering once, not at each function in that package. That description lives in the doc for sort.Interface.Less, so I'm not sure where that doc would live here. Maybe just in the package doc?

@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels May 12, 2023
@ianlancetaylor ianlancetaylor added this to the Backlog milestone May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants