Background: The usual advice in Go is to write code that does not distinguish nil slices from non-nil empty slices. However, the fundamental operations on slices all document their behavior with respect to nil, and on occasion these details matter. For example:
- []T(nil) is nil
- T{} is non-nil
- x[:0] preserves the nilness of x.
- append(x, y...) preserves the nilness of x if the result is empty. (The append built-in is defined in terms of the reslice operation above.)
Given that the slices package is intended to replace many low-level uses of x[::] and append, such as for cloning, concatenation, and clipping, I think it too should document its behavior w.r.t. nil. Its current behavior, though undocumented, is:
- slices.Clip(x) preserves the nilness of x.
- slices.Clone(s) preserves the nilness of x.
- slices.Concat(slices...) is nil iff the result is empty. (This is a regrettable inconsistency, but one I suspect it is too late to change.)
Proposal: The documentation of the slices package explicitly documents (and tests) the current behavior of each function when given nil arguments.
Comment From: gopherbot
Change https://go.dev/cl/671996 mentions this issue: slices: document and test nilness behavior of all functions
Comment From: aclements
It would be nice if we could make this more consistent, but people are certainly already depending on the current behavior, so the best we can do is to document the current behavior.
Comment From: aclements
Based on the discussion above, this proposal seems like a likely accept. — aclements for the proposal review group
The proposal is to document the existing behavior of all functions in the slices package with respect to preserving nilness.
Comment From: andig
It would be nice if we could make this more consistent, but people are certainly already depending on the current behavior, so the best we can do is to document the current behavior.
Good question. Does the compatibility promise cover implied, undocumented behavior? If not, future consistency for all users may yield higher than changing a potentially unintended behavior.
Comment From: aarzilli
The difference between Concat and append inconsistent, to me: the first argument of appened is special because it is mutated whereas Concat doesn't mutate anything.
Comment From: aclements
Good question. Does the compatibility promise cover implied, undocumented behavior? If not, future consistency for all users may yield higher than changing a potentially unintended behavior.
It's ultimately a judgement call, but in practice we lean heavily toward maintaining existing behavior, even if undocumented. Changing the behavior here is likely to lead to extremely subtle bugs in existing code, where people are unknowingly depending on this behavior. (IMO, the original sin was having both nil and non-nil empty slices, but that ship sailed a long time ago!)
Comment From: aclements
No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — aclements for the proposal review group
The proposal is to document the existing behavior of all functions in the slices package with respect to preserving nilness.