Add accessor with bounds check to Array

dennisvennink · 2018-12-02 16:15:02 UTC

I really like this idea.

Regular subscripts:

subscript(parameter:): xs[parameter: …]
subscript(_:): xs[…]

Failable subscripts:

subscript?(parameter:): xs[parameter: …?]
subscript?(_:): xs[…?]

It:

has got a precedent in init? and sets a precedent for other languages with first-class nullability to follow;
is user extendable, I can already think of a use case using Range, for instance;
is fluent, e.g., given xs[xs.startIndex?]: reads as "from xs return the element associated with startIndex if it exists, otherwise return nil;
fits naturally into ?(_:), !(_:) and ??(_:_:);
is consise.

DevAndArtist · 2018-12-02 16:19:44 UTC

Not sure if intended or not, but you got the labels wrong. Unfortunately subscripts do not follow the same labeling rules as functions.

subscript(parameter: ...) would result into value[...], while subscript(label parameter: ...) results in value[label: ...].

dennisvennink · 2018-12-02 16:41:05 UTC

Ah yes. I'm using the shorthand variant. You can find these in Xcode's Developer Documentation.

I have no idea if these have ever been formalised. It also doesn't say anything about whether it's a getter or a setter for instance.

lostatseajoshua · 2018-12-02 17:08:20 UTC

Will it include a subscript and a function, not just one of them?

My proposal is to include both a function and subscript to reflect the current behavior.

The existing regular subscript is both safe and bounds checked

Good point. Yes, you are correct the correct implementation is bounds checked I am looking to add a safe non-trapping GET only subscript/function. It will return nil instead throwing a fatal error.

lostatseajoshua · 2018-12-02 17:23:58 UTC

@DevAndArtist, @dennisvennink. A failable subscript may be the best suggestion that encompasses all the ideas everyone has thrown out. Instead of worrying about getting the parameter name perfectly correct, a nil subscript will mirror the functionality of a failable init. It implies non-fatal access. I do think it should be a GET only subscript. I doesn't make sense to have a failable subscript with SET on it.
I am trying to keep the proposal from growing too big in scope for a small feature but I believe this will do the best job on enabling the functionality and growing other possibilities.

extension Collection {
    public subscript?( position: Index) -> Element? {
        get {
            return isValidIndex(position) ? self[position] : nil
        }
    }
   
    private func isValidIndex(_ position: Index) -> Bool {
        return position >= startIndex && position < endIndex
    }
}

let array = [1, 2, 3]
array?[0] // Optional(1)
array?[100] // nil
array?[1]?.advanced(by: 1)

The failable subscript should add a ? before the subscript [] access. ex: array?[0], I am not sure this is possible but it be the best way for this to work, if not then we would be back to square one with finding a naming convention for the parameter.

DevAndArtist · 2018-12-02 17:40:43 UTC

The code sample you just posted should not work that way and as I mentioned in my first post in this thread I'd be strongly against it. Furthermore doing something like this would be a catastrophic source breaking change.

I do not like adding a direct subscript overload to Collection, because subscripts are not discoverable. Adding a simple view type (similar to what a Slice is) can provide additional information and makes it not only more convenient to use but also extensible for the future.

In your example the there is also a typo:

-> Element?
          ^
// should be just or you'll return `Element??` otherwise
-> Element

Also you are using a failable subscript as it was optional chaining. It was not my intention to pitch it like that. init? does not work like that, so shouldn't subscript?, or it will become ambiguous at some point.

The failable subscript would not make any sense if we're not going to allow the setter, because otherwise you simply don't need a failable subscript anymore. However if you have a failable subscript you can conditionally create a settable but failable subscript on the view type which is statically type safe and eliminates the issue with assigning a nil to a non-nil index of a mutable collection (which is a no-op).

struct SomeView<T> where T : Collection {
  // Since it potentially can fail, the return type is
  // automatically wrapped into an Optional.
  subscript?(index: T.Index) -> T.Element {
    get { ... }
  }
}

extension SomeView where T : MutableCollection {
  // The getter is similar to the get-only behavior for
  // a simple collection. However the setter requires
  // statically the user to provide `T.Element` not
  // `T.Element?`.
  subscript?(index: T.Index) -> T.Element {
    get { ... }
    set { ... }
  }
}

I think something like this would be by far a superior solution to this general problem. I also assume that such a view type will most likely play well with some of the concepts from the Ownership manifesto.

That said, I don't want to bombard this thread with simply a vision of mine. If you want a simpler solution, go for it, but I'd probably be against it during the review, because I don't think it's worth it in the long term.

P.S.: A small tip, there exists an equivalent to isValidIndex already which avoids the noise of manually verifying the index bounds.

// If `true` it's safe to access the collection with the given index.
collection.indices.contains(yourIndex)

dennisvennink · 2018-12-02 19:00:54 UTC

Optional chaining (also see safe navigation operator) disallows adding a ? before the [ and after the ].

This means we either have to put ? after the [ or before the ]; so xs[?xs.startIndex] versus xs[xs.startIndex?].

Here's the grammar for the latter:

_{GRAMMAR OF A POSTFIX EXPRESSION}

…
postfix-expression → subscript-expression
postfix-expression → failable-subscript-expression

_{GRAMMAR OF A SUBSCRIPT EXPRESSION}

subscript-expression → postfix-expression [ function-call-argument-list ]
failable-subscript-expression → postfix-expression [ function-call-argument-list ?]

AFAICS, Optional chaining shouldn't cause any ambiguities.

I think an explicit design is better. We could implement it in such a way that it provides an error fixit for when the return type is not an Optional. So Element? would then be correct.

Wouldn't this make it less discoverable? In addition to calling the failable subscript, the user also has to call the view property.

DevAndArtist · 2018-12-02 19:16:40 UTC

Not sure what you mean here. This code is valid today:

var array: [Int]? = nil
array?[420] = 0

Combined with a 'direct' failable subscript and the previously pitched design by @lostatseajoshua it gets quickly out of control.

// If not nil assign `1` to the index `420` if the index is not out of range.
array??[420] = 1

A view would make a little more readable.

array?.some_view[420] = 1

Well since you're not required to use an explicit ? in case of init? I think that a failable subscript shouldn't change that behavior because it will start to feel like optional chaining which really it isn't.

struct Test {
  init?() { return nil }
}
let test = Test.init() // it's not `init?()`

Sure one can argue that init is simply

static func `init`() -> Type

// or
static func `init`() -> Type?

but it only works one way. A subscript has two sides and to solve the issue of this thread I'd go a slightly different direction on how to declare a failable subscript in regard to the returned type.

Here I have to disagree because the failable subscript, as I imagine it, should solve the issue with the setter which is required to be a different type then the getter returns. The getter always wraps the returned type one level deeper into an Optional while in contrast the setter is required to be the Wrapped type. If you explicitly make it also Element? then it start to read strangely and subscript? kind of looses it's purpose.

I'd say no, because by discoverability I only meant that there is no autocompletion support for subscripts and it's super hard to tell which parameters and labels are required or even possible. A view on the other hand will fake a named subscript and since the default behavior of the subscript on the Collection types won't change we can build on additive features. A view also can be further extended if we ever decide to require more similar capabilities. That would group the API nicely together.

Those are my personal ideas. If the community does not like them, so bet it. ;) If someone can provide an even nicer approach, I'd support it.

dennisvennink · 2018-12-02 20:37:30 UTC

I meant that it's already in use for Optional chaining, which prohibits using ? before the [ and after the ] in the context of a failable subscript.

Then how can we discern it from a plain old subscript when used as an expression on a type that has both a subscript and a subscript? in a type inferred context?

Good point. I imagined that failable subscripts only allows getters.

Hypothetically, what would this then do?

var xs = [0, 1, 2]

xs[3?] = 42 // Or any other syntax.

Swift's assignment operator currently returns Void for all assignments:

print(xs[0] = 42)
// Prints "()"

Does it return nil? If so, then Element? would be fitting. But do we want a Swift where we're doing if lets on assignments? (Getting flashbacks to C here.)

If not, then what differentiates it from a plain old subscript setter?

Fair enough. But this is more of an issue that concerns IDEs. We shouldn't design our APIs around their current limitations.

Same here.

Edit: Gave it some more thought. I doubt failable subscripts and other contorted efforts to cram ? into a solution do the language any good. But I also don't think implementing it as a subscript with a named labeled parameter suits it. A throwable subscript—along with try?—does and I want to highlight @Jon_Hull's excellent post about it.

lostatseajoshua · 2018-12-02 21:27:21 UTC

collection.indicies.contains(yourIndex)

The runtime is O(n). This would cause a hit on performance. A simple if check of the bounds would yield better performance on large arrays.

https://developer.apple.com/documentation/swift/defaultindices/1784067-contains

Dante-Broggi · 2018-12-02 22:09:10 UTC

However, "A simple if check of the bounds" is incorrect in general. For example, a String.Index can have valid values between characters (e.g. from the .utf8 view) which are invalid indices for a given string.

cukr · 2018-12-02 22:51:20 UTC

Array.Indices is a Range<Int> which implements contains in O(1)

github.com

apple/swift/blob/8db57749c45da203359a390c8606bd5cf2e652fa/stdlib/public/core/Range.swift#L166


/// within the range.
///
/// Because `Range` represents a half-open range, a `Range` instance does not
/// contain its upper bound. `element` is contained in the range if it is
/// greater than or equal to the lower bound and less than the upper bound.
///
/// - Parameter element: The element to check for containment.
/// - Returns: `true` if `element` is contained in the range; otherwise,
///   `false`.
@inlinable
public func contains(_ element: Bound) -> Bool {
  return lowerBound <= element && element < upperBound
}


/// A Boolean value indicating whether the range contains no elements.
///
/// An empty `Range` instance has equal lower and upper bounds.
///
///     let empty: Range = 10..<10
///     print(empty.isEmpty)
///     // Prints "true"

jawbroken · 2018-12-03 05:27:43 UTC

I don't think you should base your design around current implementation deficiencies of the language support in one particular IDE. To me the scope of this thread is getting a little out of hand. A labelled get-only subscript would be a great solution here and comes with none of the confusion that supporting a setter would bring. As for what that label should be, it seems like this is a good starting point:

quangDecember · 2018-12-03 11:01:24 UTC

+1 for [checked: ] or [safe: ]
standard library should allow such safe options as well

Jens · 2018-12-03 13:27:00 UTC

One thing is clear from this thread: Swift fails to communicate that its regular Array subscript is checked (and thus safe).

Nevin · 2018-12-03 13:36:22 UTC

Or, alternatively, many people who are not intimately familiar with the philosophy behind Swift’s design, use the word “safe” to mean “won’t crash the application”.

benrimmington · 2018-12-03 13:43:14 UTC

The documentation (for Array and Collection) refers to "valid" indices. Has the following argument label been suggested yet?

extension Array {
  public subscript(validating index: Int) -> Element? {
    guard indices.contains(index) else { return Element?.none }
    return self[index]
  }
}

// Example One
let strings = ["A", "B"]
assert(strings[validating: 0] == "A")
assert(strings[validating: 1] == "B")
assert(strings[validating: 2] == nil)

// Example Two
let optionalDoubles: [Double?] = [Double.pi, nil]
assert(optionalDoubles[validating: 0] == Double??.some(Double.pi))
assert(optionalDoubles[validating: 1] == Double??.some(.none))
assert(optionalDoubles[validating: 2] == Double??.none)

Fryie · 2018-12-03 13:44:35 UTC

I'd argue it's probably because of using "safe" for "memory-safe", whereas a lot of people, especially from other language, might rather understand it in terms of "type-safe" (which is more closely related to "won't crash the application / will remove runtime errors").

Jens · 2018-12-03 14:02:58 UTC

ContiguousArray, Array, ArraySlice and Slice also have range-taking subscripts:

let a = ["a", "b", "c"]
let b = a[1..<3]
let c = b[ ..<2]
print(c) // ["b"]

Should there be optional-returning-instead-of-trapping-on-out-of-bounds read-only variants for them too?

dennisvennink · 2018-12-03 15:17:53 UTC

Jon_Hull:

I've mentioned it in a couple of other threads, but once we get throwing subscripts, there is a general solution that would apply well in this case.

That solution is the addition of throws! and throws? to the language. When used instead of throws , they would act as if there is an implicit try! or try? in front of the call respectively. Those implicit trys can be overridden by explicit ones.

Thus, in this case, the subscript would be marked throws! and it would throw an error instead of trap... but that error would then trap because of the implicit try! , so it would behave exactly the same as it always has. You could easily get optional behavior by using try? . Hence:
let elem = array[index] //Traps as usual

let optElem = try? array[index] //Returns an optional instead of trapping

let elem = try array[index] //Throws outOfBoundsError 

If I understood you correctly then this means we can retrofit throws? into Dictionary.subscript(_:) as well.

So not only do we solve the main issue of this thread, we also solve the discrepancy between both Indexed and Keyed subscripts when being explicit:

let array = [Int]() // Or any `Collection`

do { print(try array[0]) } catch {} // Throws
print(try? array[0]) // Prints "nil"
print(try! array[0]) // Traps

let dictionary = [Int: Int]()

do { print(try dictionary[0]) } catch {} // Throws
print(try? dictionary[0]) // Prints "nil"
print(try! dictionary[0]) // Traps

Existing code will not break:

print(array[0]) // Traps
print(dictionary[0]) // Prints "nil"