Behaviour of Collection.index(_:offsetBy:limitedBy:)

dennisvennink · 2018-12-26T14:29:49.109Z

The value passed as distance must not offset i beyond the bounds of the collection, unless the index passed as limit prevents offsetting beyond those bounds.^Source

But its behaviour seems to vary across types, with Array being the noticeable outlier:

let xs = [0, 1, 2]

_ = xs.index(xs.endIndex, offsetBy: 1, limitedBy: xs.startIndex)
// Prints "Optional(4)"

let xs = 0...2

_ = xs.index(xs.endIndex, offsetBy: 1, limitedBy: xs.startIndex)
// Crashes with message "Fatal error: Advancing past end index"

let xs = Set([0, 1, 2])

_ = xs.index(xs.endIndex, offsetBy: 1, limitedBy: xs.startIndex)
// Crashes with no message

let xs = [0: "a", 1: "b", 2: "c"]

_ = xs.index(xs.endIndex, offsetBy: 1, limitedBy: xs.startIndex)
// Crashes with no message

let xs = [0: "a", 1: "b", 2: "c"].keys

_ = xs.index(xs.endIndex, offsetBy: 1, limitedBy: xs.startIndex)
// Crashes with no message

To me it seems like it currently follows the following behaviour:

Trap when the offset i does not pass limit, but is out of bounds.
return nil when the offset i passes limit, regardless of whether or not it is out of bounds.
return an Optional value if it doesn't pass limit and is in bounds.

Is this correct? And if so, wouldn't Array's implementation be incorrect then?

Lantua · 2018-12-26T16:53:04.574Z

1. should be correct, with the same reasoning as Array.subscript returning non-optional.
2. somewhat troubles me in the case where result should go out of bound had it not been the limit boundary, but I can see this will be used extensively in low-level lib, so this shall pass...?
3. is, well, yeah.

We should file report for Array. Should we also file one for Set, Dictionary, Dictionary.Keys for crashing without message?

Ben_Cohen · 2018-12-26T20:17:34.322Z

dennisvennink:

_ = xs.index(xs.endIndex, offsetBy: 1, limitedBy: xs.startIndex)

I think there should probably be a precondition here that the starting and ending point be in the right order for the direction of the offset – so this ought to trap. @moiseev was looking at this code a while back when conditional conformance was first introduced, he might have a view.

Note Set and Dictionary are forward-only, so cannot take a negative offset and would always trap if the starting point was before the end point.

moiseev · 2019-01-02T19:46:27.830Z

Inside Arrays implementation of this method there is a comment:

// NOTE: Range checks are not performed here, because it is done later by
// the subscript function.

In general, this is true for the majority (if not all) index manipulation code inside Array. Range checks are only performed on subscripting. There's been an argument for a while, whether we should use the same approach for all the collection types, which would make the behavior uniform. But it is not clear whether it is possible to achieve as not all the collections use integer indices: what would one return when advancing past endIndex in ClosedRange?

I am a little surprised that ClosedRange provides the message whereas Dictionary does not. Glancing through the code reveals they both use _precondition / _preconditionFailure that should only produce a message in debug builds of the standard library.

Michael_Ilseman · 2019-01-02T20:32:16.357Z

moiseev:

_precondition / _preconditionFailure that should only produce a message in debug builds of the standard library.

Both _precondition and _preconditionFailure should still trigger on optimized/no-assertions stdlib and even user code. See here

moiseev · 2019-01-02T21:27:56.780Z

And they do both trigger, I was talking about the message. The message, I thought, would not appear in the non-debug stdlib build. But a little experimentation shows I was wrong.

moiseev · 2019-01-02T21:29:38.075Z

@dennisvennink Both Set and Dictionary examples from your post result in "Fatal error: Attempting to access Dictionary elements using an invalid index" error message on freshly built master.

dennisvennink · 2019-01-02T22:13:25.313Z

I forgot to mention that this behaviour occurs in the version of Swift that ships with Xcode 10.1. The latest development toolchain I have installed here from mid-December prints the messages you mentioned.