Fix range relative(to:)

[gpambrozio]

I noticed that if you have a partial range and you call relative(to:) with a collection that does not contain the range's specified bound you get a fatalError

Resolves SR-10299.

[theblixguy]

cc @airspeedswift

[jrose-apple]

I think the fatalError is correct: the range is out of bounds and the programmer should know about it. relative(to:) isn't clamp.

[gpambrozio]

I would say that this should at the very minimum be in the documentation of the method then.

Because it throws a fatalError it means that in most cases the programmer needs to do a check (like I am in some of my code now) before using the method, making, imho, for uglier code. In my code I created this for example:

extension RangeExpression {
    public func safeRelative<C>(to collection: C) -> Range<Self.Bound> where C : Collection, Self.Bound == C.Index {
        guard contains(collection.startIndex) || contains(collection.endIndex)
            else { return collection.startIndex..<collection.startIndex }
        return relative(to: collection)
    }
}

But now everyone needs to remember to not use relative(to:) and use this method instead.

I also don't see a case where throwing a fatalError would be preferable to just returning a reasonable value. But that might be my lack of imagination.

[xwu]

The existing behavior is correct. An out-of-bounds range is a logic error which is fatal in Swift; it is in fact the intention that programmers must check for that condition before calling this method. There are methods to clamp a range but relative(to:) is not one of them.

You will see that everywhere in the standard library, logic errors are handled by immediately halting execution with a fatal error. This is what the language considers safe: it is proceeding despite a logic error that is unsafe.

[gpambrozio]

Can we at least have the documentation for this method to be explicit about this behavior? I can change the PR and remove all these changes and add to the documentation instead if that's OK.

[xwu]

There are no behaviors specific to this function to be documented: as I mentioned, this is a general principle of how Swift handles invalid indices.

Where a function has unique preconditions, these are documented in the standard library. Here, as you can see, this function doesn’t have any preconditions of its own.

True, some preconditions that arise from non-obvious implementation details may be worth documenting transitively, but what happens in Swift when attempting to form an out-of-hounds range isn’t unique at all to this function. Documenting all such preconditions transitively would drown out the preconditions that are unique.

[ataibarkai]

The docs currently say the following about RangeExpression.relative(to:):

Returns the range of indices described by this range expression within the given collection.

Wouldn't we want to at the very least modify that?

I also see the point about relative(to:) being different from clamp, but the docs do not make that clear.

Unlike in most situations that concern out-of-bounds access, there is a valid range of indices that satisfies this method's description (even when the indices described by the RangeExpression don't overlap with the Collection's indices): an empty range.

[xwu]

I think the description is unambiguous. Note that the word “overlaps” is nowhere to be found. No range—not even an empty range—is “described by” a range expression with bounds that do not exist within the collection.

But, even if it were not completely clear, the usual behavior everywhere in the standard library is a fatal error in the case of invalid indices. Every alternative behavior (e.g., clamping) is explicitly documented or even labeled in the API itself because it is less safe.

[gpambrozio]

imho I think having more information is better than less. Mentioning in the doc something like: "might raise a fatalError in case the collection doesn't contain the boundary" is very little more and might help other developers that, like me, didn't realize this was a possibility here.

Keep in mind that not every swift developer is that familiar with these details and since the language is so safe to begin with we sometimes relax and trust the compiler a bit much.

[xwu]

I think definitely we need to improve our education about how Swift handles logic errors, but I would reiterate that I don’t think it belongs on the documentation for this method: if added here it would need to be added for almost every method and becomes noise.

To repeat myself again, it is precisely the fact that Swift stops execution in the case of a fatal error that makes the language safe. It is continuing execution that is unsafe.

[CodaFi]

It's been a few months since there was movement on this pull request. If you wish to pursue it, please reopen it and address the review feedback.