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

[stdlib] SR-361: Implement SE008: Add Lazy flatMap for Seq of Optionals #2461

Merged
merged 5 commits into from May 11, 2016

Conversation

russbishop
Copy link
Contributor

@russbishop russbishop commented May 10, 2016

What's in this pull request?

  • Added LazyFilterRandomAccessCollection to the generated boilerplate
  • Added flatMap<U>(transform: (Element) -> U?) extensions for sequences and the various kinds of collections
  • Added tests of the nil-coalescing (filter and map are already tested elsewhere and flatMap is just composing them)

Resolved bug number: (SR-361)


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.

https://bugs.swift.org/browse/SR-361

Adds required LazyFilterRandomAccessCollection and the flatMap extensions.
@gribozavr
Copy link
Collaborator

@russbishop Thank you!

Added LazyFilterRandomAccessCollection to the generated boilerplate

This wasn't a part of the proposal. This type was intentionally omitted because moving an index in a filtered collection can't be implemented in O(1) -- moving an index n steps requires evaluating the predicate n times.


/// Returns a `LazyMapSequence` containing the concatenated non-nil
/// results of mapping transform over this `Sequence`. The elements of
/// the result are computed lazily, each time they are read.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The summary of the doc comment should be a single sentence. CC @natecook1000

Copy link
Member

Choose a reason for hiding this comment

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

The second sentence could just be removed, since that's covered by the Lazy... type of the return.

@gribozavr
Copy link
Collaborator

@swift-ci Please test

@russbishop
Copy link
Contributor Author

@gribozavr That makes sense now that I think about it. It seems like the overload for BidirectionalCollection should be sufficient then. In that case there are only two so I'll leave it out of gyb.

I'll also fixup the tests and pull in that dataset.

@russbishop
Copy link
Contributor Author

@gribozavr I reverted the LazyFilterRandomAccessCollection changes and pushed some commits to address review comments. All tests pass locally, including validation-test/stdlib/CollectionType.swift.gyb which seems to be where that 1_stdlib/Inputs/flatMap.gyb template is getting used.

@gribozavr
Copy link
Collaborator

@russbishop Thank you!

@gribozavr
Copy link
Collaborator

@swift-ci Please test

/// - Parameter transform: A closure that accepts an element of this
/// sequence as its argument and returns an optional value.
@warn_unused_result
public func flatMap<U>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you change U to ElementOfResult?

If you feel like doing so, could you also change the names of generic type parameters in the eager flatMap overloads (stdlib/public/core/SequenceAlgorithms.swift.gyb) to SegmentOfResult and ElementOfResult?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's no problem

@gribozavr
Copy link
Collaborator

@russbishop Thank you! The CI failures are unrelated. I just have two style comments, and we should be good to merge your PR.

@russbishop
Copy link
Contributor Author

@gribozavr Pushed rename of generic arguments

@gribozavr
Copy link
Collaborator

@swift-ci Please test and merge

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.

None yet

4 participants