Skip to content

Index offsetting for Product2 #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 12, 2021
Merged

Index offsetting for Product2 #66

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
143d766
Index offsetting for Product2
timvermeulen Oct 15, 2020
4cef40c
Rename convertIndex to normalizeIndex
Jan 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
239 changes: 229 additions & 10 deletions Sources/Algorithms/Product.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
//
//===----------------------------------------------------------------------===//

/// A sequence that represents the product of two sequence's elements.
/// A sequence that represents the product of two sequences' elements.
public struct Product2<Base1: Sequence, Base2: Collection> {
/// The outer sequence in the product.
public let base1: Base1
Expand Down Expand Up @@ -119,17 +119,27 @@ extension Product2: Collection where Base1: Collection {
Index(i1: base1.endIndex, i2: base2.startIndex)
}

@inlinable
public subscript(position: Index) -> (Base1.Element, Base2.Element) {
(base1[position.i1], base2[position.i2])
}

/// Forms an index from a pair of base indices, normalizing
/// `(i, base2.endIndex)` to `(base1.index(after: i), base2.startIndex)` if
/// necessary.
@usableFromInline
internal func normalizeIndex(_ i1: Base1.Index, _ i2: Base2.Index) -> Index {
i2 == base2.endIndex
? Index(i1: base1.index(after: i1), i2: base2.startIndex)
: Index(i1: i1, i2: i2)
}

@inlinable
public func index(after i: Index) -> Index {
precondition(i.i1 != base1.endIndex, "Can't advance past endIndex")
let newIndex2 = base2.index(after: i.i2)
return newIndex2 == base2.endIndex
? Index(i1: base1.index(after: i.i1), i2: base2.startIndex)
: Index(i1: i.i1, i2: newIndex2)
return normalizeIndex(i.i1, base2.index(after: i.i2))
}

// TODO: Implement index(_:offsetBy:) and index(_:offsetBy:limitedBy:)

@inlinable
public func distance(from start: Index, to end: Index) -> Int {
guard start.i1 <= end.i1
Expand Down Expand Up @@ -183,10 +193,219 @@ extension Product2: Collection where Base1: Collection {
+ left
}
}

public func index(_ i: Index, offsetBy distance: Int) -> Index {
guard distance != 0 else { return i }

return distance > 0
? offsetForward(i, by: distance)
: offsetBackward(i, by: -distance)
}

public func index(
_ i: Index,
offsetBy distance: Int,
limitedBy limit: Index
) -> Index? {
if distance >= 0 {
return limit >= i
? offsetForward(i, by: distance, limitedBy: limit)
: offsetForward(i, by: distance)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found these ternaries confusing until I went and read the documentation for index(_:offsetBy:limitedBy:), in particular:

  ///   - limit: A valid index of the collection to use as a limit. If
  ///     `distance > 0`, a limit that is less than `i` has no effect.
  ///     Likewise, if `distance < 0`, a limit that is greater than `i` has no
  ///     effect.

I don't know if we should add a comment inline to this effect, or if it's reasonable to assume that if you're reading an implementation of this method you should be familiar with the expected behavior.

} else {
return limit <= i
? offsetBackward(i, by: -distance, limitedBy: limit)
: offsetBackward(i, by: -distance)
}
}

@inlinable
public subscript(position: Index) -> (Base1.Element, Base2.Element) {
return (base1[position.i1], base2[position.i2])
@usableFromInline
internal func offsetForward(_ i: Index, by distance: Int) -> Index {
guard let index = offsetForward(i, by: distance, limitedBy: endIndex)
else { fatalError("Index is out of bounds") }
return index
}

@usableFromInline
internal func offsetBackward(_ i: Index, by distance: Int) -> Index {
guard let index = offsetBackward(i, by: distance, limitedBy: startIndex)
else { fatalError("Index is out of bounds") }
return index
}

@usableFromInline
internal func offsetForward(
_ i: Index, by distance: Int, limitedBy limit: Index
) -> Index? {
assert(distance >= 0)
assert(limit >= i)

if limit.i1 == i.i1 {
// Delegate to `base2` if the offset is limited to `i.i1`.
//
// i.i2 limit.i2
// v v
// i.i1 > [x x x|x x x x x x|x x x]

return base2.index(i.i2, offsetBy: distance, limitedBy: limit.i2)
.map { i2 in Index(i1: i.i1, i2: i2) }
}


if let i2 = base2.index(i.i2, offsetBy: distance, limitedBy: base2.endIndex) {
// `distance` does not overflow `base2[i.i2...]`.
//
// i.i2 i2
// v v
// i.i1 > [x x x|x x x x x x|x x x]
// [ |> > > > > >| ] (`distance`)

return normalizeIndex(i.i1, i2)
}

let suffixCount = base2[i.i2...].count
let remaining = distance - suffixCount
let nextI1 = base1.index(after: i.i1)

if limit.i1 == nextI1 {
// Delegate to `base2` if the offset is limited to `nextI1`.
//
// i.i2
// v
// i.i1 > [x x x|x x x x x x x x x]
// nextI1 > [x x x x x x x x x|x x x]
// ^
// limit.i2

return base2.index(base2.startIndex, offsetBy: remaining, limitedBy: limit.i2)
.map { i2 in Index(i1: nextI1, i2: i2) }
}

if let i2 = base2.index(base2.startIndex, offsetBy: remaining, limitedBy: i.i2) {
// `remaining` does not overflow `base2[..<i.i2]`.
//
// i.i2
// v
// i.i1 > [x x x x x x x x x|x x x]
// [ |> > >] (`suffixCount`)
// [> > >| ] (`remaining`)
// nextI1 > [x x x|x x x x x x x x x]
// ^
// i2

return Index(i1: nextI1, i2: i2)
}

let prefixCount = base2[..<i.i2].count
let base2Count = prefixCount + suffixCount
let base1Distance = remaining / base2Count

guard let i1 = base1.index(nextI1, offsetBy: base1Distance, limitedBy: limit.i1)
else { return nil }

// The distance from `base2.startIndex` to the target.
let base2Distance = remaining % base2Count

let base2Limit = limit.i1 == i1 ? limit.i2 : base2.endIndex
return base2.index(base2.startIndex, offsetBy: base2Distance, limitedBy: base2Limit)
.map { i2 in Index(i1: i1, i2: i2) }
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my main question with this patch is how much do these special cases in offsetForward and offsetBackward buy us? It's a lot to hold in your head.

Can we get away with just the main "loop"?

internal func offsetForward(
  _ i: Index, by distance: Int, limitedBy limit: Index
) -> Index? {
  assert(distance >= 0)
  assert(limit >= i)

  let suffixCount = base2[i.i2...].count
  let remaining = distance - suffixCount
  let nextI1 = base1.index(after: i.i1)
  let prefixCount = base2[..<i.i2].count
  let base2Count = prefixCount + suffixCount
  let base1Distance = remaining / base2Count

  guard let i1 = base1.index(nextI1, offsetBy: base1Distance, limitedBy: limit.i1)
    else { return nil }

  // The distance from `base2.startIndex` to the target.
  let base2Distance = remaining % base2Count

  let base2Limit = limit.i1 == i1 ? limit.i2 : base2.endIndex
  return base2.index(base2.startIndex, offsetBy: base2Distance, limitedBy: base2Limit)
    .map { i2 in Index(i1: i1, i2: i2) }
}


@usableFromInline
internal func offsetBackward(
_ i: Index, by distance: Int, limitedBy limit: Index
) -> Index? {
assert(distance >= 0)
assert(limit <= i)

if limit.i1 == i.i1 {
// Delegate to `base2` if the offset is limited to `i.i1`.
//
// limit.i2 i.i2
// v v
// i.i1 > [x x x|x x x x x x|x x x]

return base2.index(i.i2, offsetBy: -distance, limitedBy: limit.i2)
.map { i2 in Index(i1: i.i1, i2: i2) }
}

if let i2 = base2.index(i.i2, offsetBy: -distance, limitedBy: base2.startIndex) {
// `distance` does not underflow `base2[..<i.i2]`.
//
// i2 i.i2
// v v
// i.i1 > [x x x|x x x x x x|x x x]
// [ |< < < < < <| ] (`distance`)

return Index(i1: i.i1, i2: i2)
}

let prefixCount = base2[..<i.i2].count
let remaining = distance - prefixCount
let previousI1 = base1.index(i.i1, offsetBy: -1)

if limit.i1 == previousI1 {
// Delegate to `base2` if the offset is limited to `previousI1`.
//
// limit.i2
// v
// previousI1 > [x x x|x x x x x x x x x]
// i.i1 > [x x x x x x x x x|x x x]
// ^
// i.i2

return base2.index(base2.endIndex, offsetBy: -remaining, limitedBy: limit.i2)
.map { i2 in Index(i1: previousI1, i2: i2) }
}

if let i2 = base2.index(base2.endIndex, offsetBy: -remaining, limitedBy: i.i2) {
// `remaining` does not underflow `base2[i.i2...]`.
//
// i2
// v
// previousI1 > [x x x x x x x x x|x x x]
// [ |< < <] (`remaining`)
// [< < <| ] (`prefixCount`)
// i.i1 > [x x x|x x x x x x x x x]
// ^
// i.i2

return Index(i1: previousI1, i2: i2)
}

let suffixCount = base2[i.i2...].count
let base2Count = prefixCount + suffixCount
let base1Distance = remaining / base2Count

// The distance from `base2.endIndex` to the target.
let base2Distance = remaining % base2Count

if base2Distance == 0 {
// We end up exactly between two cycles, so `base1Distance` would
// overshoot the target by 1.
//
// base2.startIndex
// v
// i1 > |x x x x x x x x x x x x] >
// ... > `base1Distance` times
// previousI1 > [x x x x x x x x x x x x] >
// i.i1 > [x x x|x x x x x x x x x]
// ^
// i.i2

if let i1 = base1.index(previousI1, offsetBy: -(base1Distance - 1), limitedBy: limit.i1) {
let index = Index(i1: i1, i2: base2.startIndex)
return index < limit ? nil : index
} else {
return nil
}
}

guard let i1 = base1.index(previousI1, offsetBy: -base1Distance, limitedBy: limit.i1)
else { return nil }

let base2Limit = limit.i1 == i1 ? limit.i2 : base2.startIndex
return base2.index(base2.endIndex, offsetBy: -base2Distance, limitedBy: base2Limit)
.map { i2 in Index(i1: i1, i2: i2) }
}
}

Expand Down