close
Skip to content

[stdlib] New indexing model: Fix LazyFilterCollection index moving#2070

Merged
moiseev merged 2 commits intoswiftlang:swift-3-indexing-modelfrom
natecook1000:nc-index-lazyfilter
Apr 7, 2016
Merged

[stdlib] New indexing model: Fix LazyFilterCollection index moving#2070
moiseev merged 2 commits intoswiftlang:swift-3-indexing-modelfrom
natecook1000:nc-index-lazyfilter

Conversation

@natecook1000
Copy link
Copy Markdown
Member

What's in this pull request?

The index moving methods weren't functioning correctly. They didn't trap on advancing past endIndex and in some cases did not terminate at all.

let a = [0, 0, 1, 0, 0, 2, 0, 0, 3, 0, 0]
let b = a.lazy.filter { $0 != 0 }
print(b.count)   // doesn't return

Resolved bug number: n/a


Before merging this pull request to apple/swift repository:

  • Test pull request on Swift continuous integration.

Triggering Swift CI

The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:

Smoke Testing

Platform Comment
All supported platforms @swift-ci Please smoke test
OS X platform @swift-ci Please smoke test OS X platform
Linux platform @swift-ci Please smoke test Linux platform

Validation Testing

Platform Comment
All supported platforms @swift-ci Please test
OS X platform @swift-ci Please test OS X platform
Linux platform @swift-ci Please test Linux platform

Note: Only members of the Apple organization can trigger swift-ci.

The index moving methods weren't functioning correctly. They didn't
trap on advancing past endIndex and in some cases did not terminate
at all.
@moiseev moiseev self-assigned this Apr 6, 2016
@moiseev
Copy link
Copy Markdown
Contributor

moiseev commented Apr 6, 2016

Here is the test that starts to fail after your change. Don't get me wrong, it's an improvement, as it used to not terminate at all ;-)

tests.test("LazyFilterCollection") {
  let xs = (0..<3).lazy.filter { $0 % 2 == 0 }
  expectEqual(0, xs[xs.startIndex])
}

The problem, I believe, is that _nextFilteredInPlace originally did not call the formSuccessor if the element under index satisfies the predicate. Now it does. So when the first element of a collection satisfies the predicate, it gets skipped.

The following test might help to find more problems like this:

tests.test("LazyFilterCollection") {
  let xs = (0..<3).lazy.filter { $0 % 2 == 0 }
  checkForwardCollection([0, 2], xs) {
    $0 == $1
  }
}

@natecook1000
Copy link
Copy Markdown
Member Author

Sorry about that! Fixed now and tested on better inputs.

@moiseev moiseev merged commit cd1bb19 into swiftlang:swift-3-indexing-model Apr 7, 2016
@moiseev
Copy link
Copy Markdown
Contributor

moiseev commented Apr 7, 2016

I ran the tests locally.
Thanks, Nate!

@natecook1000 natecook1000 deleted the nc-index-lazyfilter branch April 7, 2016 20:37
MaxDesiatov added a commit that referenced this pull request Apr 19, 2021
Use macOS 10.15 on GitHub Actions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants