SE-0447: Span: Safe Access to Contiguous Storage

Evolution Proposal Reviews
33

API nitpicks:

Index validation members

extension Span where Element: ~Copyable {
  public func boundsContain(_ index: Int) -> Bool
  public func boundsContain(_ indices: Range<Int>) -> Bool
  public func boundsContain(_ indices: ClosedRange<Int>) -> Bool
}

As the proposal admits, these are not good names. We should not add badly named interfaces to the Swift Standard Library.

I suggest we should remove these members from the Span types. Alternatively, I suggest giving them the following names:

extension Span where Element: ~Copyable {
  public func isValidIndex(_ index: Int) -> Bool
  public func isValidSubrange(_ subrange: Range<Int>) -> Bool
  public func isValidSubrange(_ subrange: ClosedRange<Int>) -> Bool
}

extension RawSpan {
  public func isValidByteOffset(_ offset: Int) -> Bool
  public func isValidByteOffsetRange(_ subrange: Range<Int>) -> Bool
  public func isValidByteOffsetRange(_ subrange: ClosedRange<Int>) -> Bool
}

The ClosedRange overloads are rubbing me the wrong way -- why do we need those? Should we also have a generic isValidSubrange that takes any RangeExpression? I think I'd be happier if we didn't have any of these.

Update: I've now remembered that I already suggested these in public, way back in June. This makes me a bit confused.

If we can only find "not ideal" names for these predicates, then I don't see why we would want to propose them as public API. After all, no preexisting standard container comes with such operations. The proposal does not justify why they'd need to be unique to Span, but neither does it mention the obvious alternative: that we'd need to add similar predicates to the UnsafeBufferPointer types, too.

I therefore suggest that these operations be removed, deferring them to a potential future proposal that properly introduces them in the general stdlib, rather than isolating their discussion to a single type. Clearly, we are not ready to do that in this proposal.

(Any adopters of Span types will be able to validate indices and offsets the obvious way -- by manually checking that they're within 0 ..< count. This is similar to how we are currently forced to implement index validation in code that deals with UnsafeBufferPointer.) (End of update.)

Structural containment checks

I second the idea of removing isWithin(_:), subsuming it into indicesWithin(_:).

However, I think the proposed behavior of the indicesWithin(_:) method is quite inconsistent with preexisting API design practice for container types. It is proposed to be a method on a container type that returns the container's index type, but the indices returned belong to some other container, not self. This is highly irregular. I believe it to be without precedent in/near the Standard Library, and I do not think we'd want to establish it as a viable pattern.

I suggest to turn the indicesWithin method inside out (swapping its self with its argument), and rename it to subrange(of:):

extension Span where Element: ~Copyable {
  /// Returns the index range where the memory of `span` is located within
  /// `self`, or `nil` if `span` is not a subspan of `self`.
  ///
  /// - Parameters:
  ///    - span: a potential subspan of `self`
  /// - Returns: A range of indices within `self` corresponding to `span`, or `nil` if there aren't any.
  /// - Complexity: O(1)
  public func subrange(of span: borrowing Self) -> Range<Int>?
}

indices(of:) would also be a viable alternative spelling for this.

This suggests byteOffsets(of:) to be the name of the analogue method on RawSpan:

extension RawSpan {
  public func byteOffsets(of span: borrowing Self) -> Range<Int>?
}

On indices

I would not automatically expect a hypothetical future container protocol that supports noncopyable conforming types (and elements) to come with an indices property.

FWIW, the very early draft we have for such a protocol does not come with one.

Part of the problem is that there is a fundamental need to avoid clashing with the indices property in Collection. Some types will want to conform to both protocols, so the noncopyable-flavored protocol can only come with indices if it can be made fully compatible with Collection's requirements. Unfortunately, the new protocol's indices type would need to be nonescapable, which makes this unlikely to work: it seems highly unlikely that Collection will ever be generalized to support nonescapable Indices associated types.

(The default value of this new, noncopyable-flavored Indices associated type would need to be a type that holds a borrow of the original generic noncopyable container. Borrows are inherently nonescapable, and so a type that contains a borrow would itself need to be nonescapable. Note that we cannot express stored borrows in today's Swift, not even with experimental feature flags.)

Therefore, while it is not entirely impossible that a new container protocol would provide something equivalent to indices, but (1) it would probably need to come with a different name, and (2) it would also require language constructs that do not exist yet.

I think this makes it a bad idea to ship Span with an indices property today. Its implementation would be effectively 0 ..< self.count; we do not urgently need a named property to avoid typing that. Once the dust has settled a bit on the new container protocols, then we may revisit this issue. (For example, if the container protocols end up not having anything like an indices view, then that makes Span free to add one.)

The lack of a practical use case

I am of course fully on board with adding the Span/RawSpan types to the Standard Library. But I question the wisdom of proposing them now: without initializers, and without any other way to produce actual span instances, the proposed span types aren't going to be actually usable for anything in practice.

Therefore, I think it would be pointless to actually add Span and RawSpan to the Standard Library in their proposed partial form. This proposal is incomplete and useless without crucial followup work that (1) properly defines/explains lifetime dependencies and (2) that introduces the missing span initializers. SE-0447 should not ship in a Swift release without that followup work.

At the moment, this proposal is a didactic crutch at best. It is going to allow the lifetimes proposal to tie its concepts to concrete motivating code examples that will be far more useful than the usual half-baked foo/bar illustrations. Those examples will breath life into Span, giving it its true purpose. (At worst, it encourages us to make premature, but binding API decisions without fully understanding the ultimate role (and unique constraints of) this type. It does not help that the proposal (for whatever reason) appears to intentionally propose suboptimal names for much of what little API it does define. :face_with_raised_eyebrow:)

The proposal suggests that it would be possible to implement withSpan/withBytes methods without the need to reason about lifetime dependencies. I don't think that is true -- in fact, I believe that such higher order functions establish far more complicated lifetime dependencies than a function that directly returns a Span. We currently lack the means to reason about these dependencies; and I don't believe it would be okay to introduce API that Swift developers cannot properly understand. (This issue also applies to the subscripts that are in the normative part of this proposal, as well as the UnsafePointer.pointee and the UnsafeBufferPointer.subscript generalizations we shipped in SE-0437. All of these have deep semantic problems, especially with noncopyable pointees/elements.)

16 Likes