Skip to content
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.

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

[SR-12881] DefaultIndices conditional conformances incomplete #55328

Closed
dabrahams opened this issue May 26, 2020 · 4 comments
Closed

[SR-12881] DefaultIndices conditional conformances incomplete #55328

dabrahams opened this issue May 26, 2020 · 4 comments

Comments

@dabrahams
Copy link
Collaborator

@dabrahams dabrahams commented May 26, 2020

Previous ID SR-12881
Radar None
Original Reporter @dabrahams
Type Bug
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug
Assignee None
Priority Medium

md5: 963ca68213205a6037a30d43644edfef

Issue Description:

This program crashes with: Fatal error: Only BidirectionalCollections can have end come before start. It should not.

extension Collection {
  func generic_distance(from i: Index, to j: Index) -> Int {
    distance(from: i, to: j)
  }
}

extension RandomAccessCollection {
  var isRandomAccess: Bool { true }
}

let r = (0...10).indices
assert(r.isRandomAccess)
print(r.distance(from: r.endIndex, to: r.startIndex))
print(r.generic_distance(from: r.endIndex, to: r.startIndex)) // trap
@NevinBR
Copy link
Contributor

@NevinBR NevinBR commented May 26, 2020

For DefaultIndices, I believe this can be fixed by implementing those methods in the extension granting Collection conformance to DefaultIndices.

The problem occurs because at the point where DefaultIndices conforms to Collection, the only available implementation for distance(from:to:) is the single-ended default version from Collection itself. So that becomes the witness for DefaultIndices.distance.

By putting a concrete implementation in that extension, the compiler will choose that version instead of the default. All the concrete version does is forward to _elements, so the correct, dynamically-dispatched method will be called.

@NevinBR
Copy link
Contributor

@NevinBR NevinBR commented May 26, 2020

I have put up a PR to fix this for DefaultIndices:

#32019

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Jun 15, 2020

Comment by Matthew Rips (JIRA)

Interesting observation:

In Swift 3.1.1, the example given does not trap. It calls to the correct implementation of `distance(from:to🙂`, and correctly prints -11 as the output for both expressions.

I ran it on 3.1.1 using the IDE at https://www.jdoodle.com/execute-swift-online/. To make it compile in 3.1.1, I needed to change `Int` to the now deprecated `Self.IndexDistance`.

@lorentey
Copy link
Member

@lorentey lorentey commented Jul 14, 2020

Merged @NevinBR's PR to resolve this.

The triplicate witnesses for these indexing operations make it tricky to perfectly forward them to another collection; but AFAICT Nevin's fix successfully resolves the (almost universal) case where all three variants share the same implementation.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants