SR-122 / CollectionsMoveIndices.swift Prototype


(Shawn Erickson) #1

First I see that Dmitri G. appears to be most involved with this yet in the
other thread I see Dmitri H. being copied. So which/both of you involved
with this effort? ...or are you one and the same given the similarity of
the name? /me hopes he doesn't look like a total idiot now

Second given commit access limits it may make sense for those of us on the
"outside" to work in same fork (e.g. added as collaborators)? It may help
avoid pull request overhead between us while work is underway? My github
identity is "shawnce". I have a fork created
https://github.com/shawnce/swift if so desired to use that as the sandbox
(set default branch to swift-3-indexing-model). ...looking for guidance on
how best to make things work efficiently.

I am digging into
https://github.com/apple/swift/blob/master/test/Prototypes/CollectionsMoveIndices.swift
to
understand the scope of the work involved.

Anyway I am looking at the current state of the code and I see things like
the following...
   @available(*, unavailable, renamed="MutableCollection")
    public typealias MutableCollectionType = MutableCollection
...so it looks like the use of Type is being dropped in the updated naming
methodology? So this obviously implies... right?
// [new] protocol BidirectionalCollection : Collection {}
// [new] protocol RandomAccessCollection : BidirectionalCollection

I expect additional – hopefully less mundane :slight_smile: – questions to popup as I
dig into things.

-Shawn

As reference Dave outlined the following in the other email thread...

···

-----
Okay, I've up a branch for you guys: swift-3-indexing-model.

* You can submit pull requests against that.

* There are corresponding branches in the swift-llvm and swift-clang
  repos that this branch will build/test against.

* The branch is based on swift-3-api-guidelines, where we're doing all
  the renaming work associated with the new guidelines; we expect to
  merge that branch into master in a few days, so basing the indexing model
  work on it should reduce conflicts when we merge this work.

Most of the stdlib team is currently occupied with other fires, but we
want to move on to work on this ASAP. If it's possible for you guys to
get it started in the meantime, that would be truly awesome.
Unfortunately the hardest part is right at the beginning.

The first step is to make the minimal changes required to get the
standard library to build after replacing the Collection protocol with
the three shown in the prototype. That step may not be very
parallelizable and might require either close coordination or for one of
you to do it alone. Tests will be horribly broken at this point, and
you may have even commented out parts of the standard library, so this
step requires intestinal fortitude.

If parts of the library have been disabled in step 1, next you can split
up the work of getting the whole library to work.

At this point you should be able to mark the old Index protocols
@unavailable and move on to fixing tests.
-----


(Austin Zheng) #2

Agreed. I created a GitHub organization
('swift-stdlib-opensource-collaborators'), and will try to invite the
non-Apple ('outsider') folks to join. Once that's happened, maybe Shawn can
move his fork under the organization, or one of us can fork the repo again.
I'm not very familiar with how orgs work, but I assume we'll be able to
push to the fork with minimal friction, and then occasionally the org
itself can make PRs to Swift proper?

Once we get this worked out I'll begin working through the first part of
Dave's tasklist.

Best,
Austin

···

On Sun, Feb 21, 2016 at 9:57 AM, Shawn Erickson <shawnce@gmail.com> wrote:

First I see that Dmitri G. appears to be most involved with this yet in
the other thread I see Dmitri H. being copied. So which/both of you
involved with this effort? ...or are you one and the same given the
similarity of the name? /me hopes he doesn't look like a total idiot now

Second given commit access limits it may make sense for those of us on the
"outside" to work in same fork (e.g. added as collaborators)? It may help
avoid pull request overhead between us while work is underway? My github
identity is "shawnce". I have a fork created
https://github.com/shawnce/swift if so desired to use that as the sandbox
(set default branch to swift-3-indexing-model). ...looking for guidance on
how best to make things work efficiently.

I am digging into
https://github.com/apple/swift/blob/master/test/Prototypes/CollectionsMoveIndices.swift to
understand the scope of the work involved.

Anyway I am looking at the current state of the code and I see things like
the following...
   @available(*, unavailable, renamed="MutableCollection")
    public typealias MutableCollectionType = MutableCollection
...so it looks like the use of Type is being dropped in the updated naming
methodology? So this obviously implies... right?
// [new] protocol BidirectionalCollection : Collection {}
// [new] protocol RandomAccessCollection : BidirectionalCollection

I expect additional – hopefully less mundane :slight_smile: – questions to popup as I
dig into things.

-Shawn

As reference Dave outlined the following in the other email thread...
-----
Okay, I've up a branch for you guys: swift-3-indexing-model.

* You can submit pull requests against that.

* There are corresponding branches in the swift-llvm and swift-clang
  repos that this branch will build/test against.

* The branch is based on swift-3-api-guidelines, where we're doing all
  the renaming work associated with the new guidelines; we expect to
  merge that branch into master in a few days, so basing the indexing model
  work on it should reduce conflicts when we merge this work.

Most of the stdlib team is currently occupied with other fires, but we
want to move on to work on this ASAP. If it's possible for you guys to
get it started in the meantime, that would be truly awesome.
Unfortunately the hardest part is right at the beginning.

The first step is to make the minimal changes required to get the
standard library to build after replacing the Collection protocol with
the three shown in the prototype. That step may not be very
parallelizable and might require either close coordination or for one of
you to do it alone. Tests will be horribly broken at this point, and
you may have even commented out parts of the standard library, so this
step requires intestinal fortitude.

If parts of the library have been disabled in step 1, next you can split
up the work of getting the whole library to work.

At this point you should be able to mark the old Index protocols
@unavailable and move on to fixing tests.
-----


(Dmitri Gribenko) #3

First I see that Dmitri G. appears to be most involved with this yet in the
other thread I see Dmitri H. being copied. So which/both of you involved
with this effort? ...or are you one and the same given the similarity of the
name? /me hopes he doesn't look like a total idiot now

I'm just one Dmitri.

Second given commit access limits it may make sense for those of us on the
"outside" to work in same fork (e.g. added as collaborators)? It may help
avoid pull request overhead between us while work is underway? My github
identity is "shawnce". I have a fork created
https://github.com/shawnce/swift if so desired to use that as the sandbox
(set default branch to swift-3-indexing-model). ...looking for guidance on
how best to make things work efficiently.

I'll check if it is possible to give you commit access.

I am digging into
https://github.com/apple/swift/blob/master/test/Prototypes/CollectionsMoveIndices.swift
to understand the scope of the work involved.

Anyway I am looking at the current state of the code and I see things like
the following...
   @available(*, unavailable, renamed="MutableCollection")
    public typealias MutableCollectionType = MutableCollection
...so it looks like the use of Type is being dropped in the updated naming
methodology? So this obviously implies... right?
// [new] protocol BidirectionalCollection : Collection {}
// [new] protocol RandomAccessCollection : BidirectionalCollection

Right. The prototype is currently written using old names.

Dmitri

···

On Sun, Feb 21, 2016 at 9:57 AM, Shawn Erickson <shawnce@gmail.com> wrote:

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Dmitri Gribenko) #4

Hi Austin, Shawn,

We're still working out the general policy for commit access for
non-Apple contributors.

I'm trying to understand the situation better -- could you explain why
pull requests present too much overhead for this project? Many Apple
engineers who have commit access find that the pull request approach
works better for their day-to-day work.

My concern is that doing this work in a parallel organization hides
this project from other contributors who might be interested. Also,
you would only get CI coverage in the primary Swift organization. In
general, creating a parallel organization sends an ambiguous message
to other people working on the project.

Furthermore, even Shawn started his work on this project with a pull
request against his fork (https://github.com/shawnce/swift/pull/1).

Could we start with pull requests against the swift-3-indexing-model
branch in the primary repository, and possibly move to direct commits
later?

Dmitri

···

On Sun, Feb 21, 2016 at 12:13 PM, Austin Zheng <austinzheng@gmail.com> wrote:

Agreed. I created a GitHub organization
('swift-stdlib-opensource-collaborators'), and will try to invite the
non-Apple ('outsider') folks to join. Once that's happened, maybe Shawn can
move his fork under the organization, or one of us can fork the repo again.

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Austin Zheng) #5

Hi Dmitri (et al),

I have no personal objection to pull requests. If PRs directly to the Swift project are the best way to do things, let's keep it that way.

Austin

···

On Feb 21, 2016, at 5:24 PM, Dmitri Gribenko <gribozavr@gmail.com> wrote:

On Sun, Feb 21, 2016 at 12:13 PM, Austin Zheng <austinzheng@gmail.com> wrote:

Agreed. I created a GitHub organization
('swift-stdlib-opensource-collaborators'), and will try to invite the
non-Apple ('outsider') folks to join. Once that's happened, maybe Shawn can
move his fork under the organization, or one of us can fork the repo again.

Hi Austin, Shawn,

We're still working out the general policy for commit access for
non-Apple contributors.

I'm trying to understand the situation better -- could you explain why
pull requests present too much overhead for this project? Many Apple
engineers who have commit access find that the pull request approach
works better for their day-to-day work.

My concern is that doing this work in a parallel organization hides
this project from other contributors who might be interested. Also,
you would only get CI coverage in the primary Swift organization. In
general, creating a parallel organization sends an ambiguous message
to other people working on the project.

Furthermore, even Shawn started his work on this project with a pull
request against his fork (https://github.com/shawnce/swift/pull/1).

Could we start with pull requests against the swift-3-indexing-model
branch in the primary repository, and possibly move to direct commits
later?

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Dave Abrahams) #6

Until we open commit access, they still need one or more repos to push to and create PRs from. Seems better for them to have an org repo for that so other collaborators have a centralized place to go for the latest non-integrated work.

···

Sent from my moss-covered three-handled family gradunza

On Feb 21, 2016, at 5:34 PM, Austin Zheng <austinzheng@gmail.com> wrote:

Hi Dmitri (et al),

I have no personal objection to pull requests. If PRs directly to the Swift project are the best way to do things, let's keep it that way.

Austin

On Feb 21, 2016, at 5:24 PM, Dmitri Gribenko <gribozavr@gmail.com> wrote:

On Sun, Feb 21, 2016 at 12:13 PM, Austin Zheng <austinzheng@gmail.com> wrote:

Agreed. I created a GitHub organization
('swift-stdlib-opensource-collaborators'), and will try to invite the
non-Apple ('outsider') folks to join. Once that's happened, maybe Shawn can
move his fork under the organization, or one of us can fork the repo again.

Hi Austin, Shawn,

We're still working out the general policy for commit access for
non-Apple contributors.

I'm trying to understand the situation better -- could you explain why
pull requests present too much overhead for this project? Many Apple
engineers who have commit access find that the pull request approach
works better for their day-to-day work.

My concern is that doing this work in a parallel organization hides
this project from other contributors who might be interested. Also,
you would only get CI coverage in the primary Swift organization. In
general, creating a parallel organization sends an ambiguous message
to other people working on the project.

Furthermore, even Shawn started his work on this project with a pull
request against his fork (https://github.com/shawnce/swift/pull/1).

Could we start with pull requests against the swift-3-indexing-model
branch in the primary repository, and possibly move to direct commits
later?

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Shawn Erickson) #7

As an FYI I have been playing around with things mostly to get my head
around the base changes and implications on the existing stdlib
implementation. It is very early still. I have no issues throwing it away
if I am way off the rails.

Review - https://github.com/shawnce/swift/pull/1

···

On Sun, Feb 21, 2016 at 4:12 PM Dmitri Gribenko <gribozavr@gmail.com> wrote:

On Sun, Feb 21, 2016 at 9:57 AM, Shawn Erickson <shawnce@gmail.com> wrote:
> First I see that Dmitri G. appears to be most involved with this yet in
the
> other thread I see Dmitri H. being copied. So which/both of you involved
> with this effort? ...or are you one and the same given the similarity of
the
> name? /me hopes he doesn't look like a total idiot now

I'm just one Dmitri.

> Second given commit access limits it may make sense for those of us on
the
> "outside" to work in same fork (e.g. added as collaborators)? It may help
> avoid pull request overhead between us while work is underway? My github
> identity is "shawnce". I have a fork created
> https://github.com/shawnce/swift if so desired to use that as the
sandbox
> (set default branch to swift-3-indexing-model). ...looking for guidance
on
> how best to make things work efficiently.

I'll check if it is possible to give you commit access.

> I am digging into
>
https://github.com/apple/swift/blob/master/test/Prototypes/CollectionsMoveIndices.swift
> to understand the scope of the work involved.
>
> Anyway I am looking at the current state of the code and I see things
like
> the following...
> @available(*, unavailable, renamed="MutableCollection")
> public typealias MutableCollectionType = MutableCollection
> ...so it looks like the use of Type is being dropped in the updated
naming
> methodology? So this obviously implies... right?
> // [new] protocol BidirectionalCollection : Collection {}
> // [new] protocol RandomAccessCollection : BidirectionalCollection

Right. The prototype is currently written using old names.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Joe Groff) #8

That's just a romanization failure. Dmitri G. and H. are the same person.

-Joe

···

On Feb 21, 2016, at 9:57 AM, Shawn Erickson via swift-dev <swift-dev@swift.org> wrote:

First I see that Dmitri G. appears to be most involved with this yet in the other thread I see Dmitri H. being copied.


(Shawn Erickson) #9

Dmitri, just checking in... I have some more time to work on things it
looks like. I will continue on Flatten.swift but that may go quickly. Just
wanted some guidance on what to work on next (or what not to work on). I
don't want to duplicate anything you may be working on. For now I will
assume I should just continue on getting things back to a compiling state
(and less on backfilling missing implementation).

Current state of my hacking...
https://github.com/apple/swift/pull/1592
https://github.com/apple/swift/pull/1588


(Dmitri Gribenko) #10

Thank you! I took a look briefly, and I have a question -- why are
you using ~Indexable as the base protocol? Indexable itself only
exists to work around a compiler limitation and implement
IndexingIterator. Otherwise we should be using Collection.

Dmitri

···

On Sun, Feb 21, 2016 at 4:21 PM, Shawn Erickson <shawnce@gmail.com> wrote:

As an FYI I have been playing around with things mostly to get my head
around the base changes and implications on the existing stdlib
implementation. It is very early still. I have no issues throwing it away if
I am way off the rails.

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Austin Zheng) #11

I copy-pasted the prototype code into Collections.swift (commenting out the old code), renamed the types that conflicted with the naming guidelines, and am going through the errors one at a time to get the project into a buildable state. This might take a few more days. Let me know if there are any objections to this approach.

Austin

···

On Feb 21, 2016, at 6:21 PM, Dave Abrahams <dabrahams@apple.com> wrote:

Until we open commit access, they still need one or more repos to push to and create PRs from. Seems better for them to have an org repo for that so other collaborators have a centralized place to go for the latest non-integrated work.

Sent from my moss-covered three-handled family gradunza

On Feb 21, 2016, at 5:34 PM, Austin Zheng <austinzheng@gmail.com> wrote:

Hi Dmitri (et al),

I have no personal objection to pull requests. If PRs directly to the Swift project are the best way to do things, let's keep it that way.

Austin

On Feb 21, 2016, at 5:24 PM, Dmitri Gribenko <gribozavr@gmail.com> wrote:

On Sun, Feb 21, 2016 at 12:13 PM, Austin Zheng <austinzheng@gmail.com> wrote:

Agreed. I created a GitHub organization
('swift-stdlib-opensource-collaborators'), and will try to invite the
non-Apple ('outsider') folks to join. Once that's happened, maybe Shawn can
move his fork under the organization, or one of us can fork the repo again.

Hi Austin, Shawn,

We're still working out the general policy for commit access for
non-Apple contributors.

I'm trying to understand the situation better -- could you explain why
pull requests present too much overhead for this project? Many Apple
engineers who have commit access find that the pull request approach
works better for their day-to-day work.

My concern is that doing this work in a parallel organization hides
this project from other contributors who might be interested. Also,
you would only get CI coverage in the primary Swift organization. In
general, creating a parallel organization sends an ambiguous message
to other people working on the project.

Furthermore, even Shawn started his work on this project with a pull
request against his fork (https://github.com/shawnce/swift/pull/1).

Could we start with pull requests against the swift-3-indexing-model
branch in the primary repository, and possibly move to direct commits
later?

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Shawn Erickson) #12

Yeah I was fairly sure that was the direction it should go. Was trying to
understand what existed and why. I was also bucketing things in a
nondestructive way in the short term until I understood more about the
existing Collection code.

Do we want a ForewardCollection like the prototype or just refocus
Collection on that with the two new subprotocols adding in the other
behaviors (existing Collection is mostly forward indexing with where on
Index type to adjust behavior).

···

On Sun, Feb 21, 2016 at 4:26 PM Dmitri Gribenko <gribozavr@gmail.com> wrote:

On Sun, Feb 21, 2016 at 4:21 PM, Shawn Erickson <shawnce@gmail.com> wrote:
> As an FYI I have been playing around with things mostly to get my head
> around the base changes and implications on the existing stdlib
> implementation. It is very early still. I have no issues throwing it
away if
> I am way off the rails.

Thank you! I took a look briefly, and I have a question -- why are
you using ~Indexable as the base protocol? Indexable itself only
exists to work around a compiler limitation and implement
IndexingIterator. Otherwise we should be using Collection.

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Dmitri Gribenko) #13

Dmitri, just checking in... I have some more time to work on things it looks
like. I will continue on Flatten.swift but that may go quickly. Just wanted
some guidance on what to work on next (or what not to work on). I don't want
to duplicate anything you may be working on. For now I will assume I should
just continue on getting things back to a compiling state (and less on
backfilling missing implementation).

Hi Shawn,

Sorry I haven't had time to work on the code or review PRs yesterday.
When the standard library compiles, we should continue with fixing the
build for other components -- the SDK overlays and unittest support
libraries, so that `build-script -R` can finish cleanly.

Current state of my hacking...
https://github.com/apple/swift/pull/1592
https://github.com/apple/swift/pull/1588

Thank you! I was reviewing these this morning, and I want to merge
them shortly.

Dmitri

···

On Wed, Mar 9, 2016 at 11:17 AM, Shawn Erickson via swift-dev <swift-dev@swift.org> wrote:

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Shawn Erickson) #14

I see a handful of issues like the following coming about because of
changes to `Range`. In the past `Range` conformed to `Collection` so
`count` came from `Collection` which returned a type of
`ForwardIndex.Distance` (e.g. `_SignedInteger`). It now comes from
`Strideable`’s `distance(to:)` which returns `Strideable.Stride` (e.g.
`SignedNumber`).

error: value of type 'C.Index.Stride' has no member 'toIntMax'
  let len = range.count.toIntMax()
            ~~~~~~^~~~~ ~~~~~~~~

Additionally when trying to provide `Strideable` optimized `Collection`
default implementations I hit issues like the following (note
`Collection.IndexDistance` minimally conforms to SignedInteger). I picked
`Strideable` as the optimization point since `RandomAccessCollection.Index`
minimally conforms to `Strideable` and `Strideable` provides the first
level of possible optimizations after `Comparable`.

extension Collection where Index : Strideable {
  public func next(i: Index) -> Index {
    return i.advanced(by: 1) <— this is ok
  }
  public func advance(i: Index, by n: IndexDistance) -> Index {
    return i.advanced(by: n) <— this errors
  }

}

error: cannot invoke 'advanced' with an argument list of type '(by:
Self.IndexDistance)'
    return i.advanced(by: n)
             ^
note: expected an argument list of type '(by: Self.Index.Stride)'
    return i.advanced(by: n)

Looking at things (see below ASCII art) I see that `_SignedInteger`
conforms to `SignedNumber `(and `SignedInteger` conforms to
`_SignedInteger`). `SignedNumber` doesn’t provide toIntMax nor can it act
as an `SignedInteger`. The use of `SignedNumber` in `Strideable` is hence
problematic. I guess the reason for `SignedNumber` is to let distances in
Strideable support non-integer types?

I am not sure the best path forward at the moment. Do I attempt to scope
things to `Strideable` where `Stride : SignedInteger` since
`Collection.IndexDistance` – as currently defined – doesn’t allow for
non-integer types? I am not sure the allowable scoping for Range in that
situation? ...or do we change `Strideable.Stride` to minimally conform to
`SignedInteger`? ...or?

Pardon my naïvety in this area of things,
-Shawn

···

————————————————————————————————————————
Equatable
  ^
Comparable
  ^
Strideable (associatedtype Stride : SignedNumber) ->
advanced(by:Self.Stride) & distance(to: Self)
  ^
  ^ SignedNumber
  ^ ^
Integer & _SignedInteger —> func toIntMax()
  ^
SignedInteger —> func toIntMax()
————————————————————————————————————————
Indexable (associatedtype Index : Comparable)
  ^
Collection (associatedtype IndexDistance : SignedInteger)
  ^
BidrectionalCollection
  ^
RandomAccessCollection (associatedtype Index : Strideable)
————————————————————————————————————————
MutableIndexable (associatedtype Index : Comparable)
  ^
  ^ Collection
  ^ ^
MutableCollection (associatedtype IndexDistance : SignedInteger)
————————————————————————————————————————


(Dmitri Gribenko) #15

I think ForwardCollection should be called just Collection, and have
two refined protocols just as you are describing it.

Dmitri

···

On Sun, Feb 21, 2016 at 4:35 PM, Shawn Erickson <shawnce@gmail.com> wrote:

Yeah I was fairly sure that was the direction it should go. Was trying to
understand what existed and why. I was also bucketing things in a
nondestructive way in the short term until I understood more about the
existing Collection code.

Do we want a ForewardCollection like the prototype or just refocus
Collection on that with the two new subprotocols adding in the other
behaviors (existing Collection is mostly forward indexing with where on
Index type to adjust behavior).

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Dave Abrahams) #16

I copy-pasted the prototype code into Collections.swift (commenting
out the old code),

Request: don't comment out old code; it just makes a mess and makes
changesets harder to analyze. The old code is still available; that's
what Git is for.

renamed the types that conflicted with the naming guidelines, and am
going through the errors one at a time to get the project into a
buildable state. This might take a few more days. Let me know if there
are any objections to this approach.

None whatsoever. If you can push your WIP to some publicly-visible
repository, maybe you could find a way to share the effort of fixing
errors with Shawn...?

···

on Sun Feb 21 2016, Austin Zheng <austinzheng-AT-gmail.com> wrote:

Austin

On Feb 21, 2016, at 6:21 PM, Dave Abrahams <dabrahams@apple.com> wrote:

Until we open commit access, they still need one or more repos to
push to and create PRs from. Seems better for them to have an org
repo for that so other collaborators have a centralized place to go
for the latest non-integrated work.

Sent from my moss-covered three-handled family gradunza

On Feb 21, 2016, at 5:34 PM, Austin Zheng <austinzheng@gmail.com> wrote:

Hi Dmitri (et al),

I have no personal objection to pull requests. If PRs directly to
the Swift project are the best way to do things, let's keep it that
way.

Austin

On Feb 21, 2016, at 5:24 PM, Dmitri Gribenko <gribozavr@gmail.com> wrote:

On Sun, Feb 21, 2016 at 12:13 PM, Austin Zheng <austinzheng@gmail.com> wrote:

Agreed. I created a GitHub organization
('swift-stdlib-opensource-collaborators'), and will try to invite the
non-Apple ('outsider') folks to join. Once that's happened, maybe Shawn can
move his fork under the organization, or one of us can fork the repo again.

Hi Austin, Shawn,

We're still working out the general policy for commit access for
non-Apple contributors.

I'm trying to understand the situation better -- could you explain why
pull requests present too much overhead for this project? Many Apple
engineers who have commit access find that the pull request approach
works better for their day-to-day work.

My concern is that doing this work in a parallel organization hides
this project from other contributors who might be interested. Also,
you would only get CI coverage in the primary Swift organization. In
general, creating a parallel organization sends an ambiguous message
to other people working on the project.

Furthermore, even Shawn started his work on this project with a pull
request against his fork (https://github.com/shawnce/swift/pull/1).

Could we start with pull requests against the swift-3-indexing-model
branch in the primary repository, and possibly move to direct commits
later?

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/

--
-Dave


(Shawn Erickson) #17

If you can see a way that I can assist Austin let me know. I was
approaching it by systematically pulling in the API and implementation in a
passive way into the existing code. Then toggling things to see the scope
of changes that would be needed. Not sure what is the best way to attack
this. I will hold on that attempt for now.

See my pull request or fork for what little I had started (if you haven't
already).

https://github.com/shawnce/swift/pull/1/files
https://github.com/shawnce/swift/

-Shawn

···

On Sun, Feb 21, 2016 at 6:45 PM Austin Zheng <austinzheng@gmail.com> wrote:

I copy-pasted the prototype code into Collections.swift (commenting out
the old code), renamed the types that conflicted with the naming
guidelines, and am going through the errors one at a time to get the
project into a buildable state. This might take a few more days. Let me
know if there are any objections to this approach.

Austin

> On Feb 21, 2016, at 6:21 PM, Dave Abrahams <dabrahams@apple.com> wrote:
>
> Until we open commit access, they still need one or more repos to push
to and create PRs from. Seems better for them to have an org repo for that
so other collaborators have a centralized place to go for the latest
non-integrated work.
>
> Sent from my moss-covered three-handled family gradunza
>
>> On Feb 21, 2016, at 5:34 PM, Austin Zheng <austinzheng@gmail.com> > wrote:
>>
>> Hi Dmitri (et al),
>>
>> I have no personal objection to pull requests. If PRs directly to the
Swift project are the best way to do things, let's keep it that way.
>>
>> Austin
>>
>>> On Feb 21, 2016, at 5:24 PM, Dmitri Gribenko <gribozavr@gmail.com> > wrote:
>>>
>>> On Sun, Feb 21, 2016 at 12:13 PM, Austin Zheng <austinzheng@gmail.com> > wrote:
>>>> Agreed. I created a GitHub organization
>>>> ('swift-stdlib-opensource-collaborators'), and will try to invite the
>>>> non-Apple ('outsider') folks to join. Once that's happened, maybe
Shawn can
>>>> move his fork under the organization, or one of us can fork the repo
again.
>>>
>>> Hi Austin, Shawn,
>>>
>>> We're still working out the general policy for commit access for
>>> non-Apple contributors.
>>>
>>> I'm trying to understand the situation better -- could you explain why
>>> pull requests present too much overhead for this project? Many Apple
>>> engineers who have commit access find that the pull request approach
>>> works better for their day-to-day work.
>>>
>>> My concern is that doing this work in a parallel organization hides
>>> this project from other contributors who might be interested. Also,
>>> you would only get CI coverage in the primary Swift organization. In
>>> general, creating a parallel organization sends an ambiguous message
>>> to other people working on the project.
>>>
>>> Furthermore, even Shawn started his work on this project with a pull
>>> request against his fork (https://github.com/shawnce/swift/pull/1).
>>>
>>> Could we start with pull requests against the swift-3-indexing-model
>>> branch in the primary repository, and possibly move to direct commits
>>> later?
>>>
>>> Dmitri
>>>
>>> --
>>> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
>>> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/
>>


(Dmitri Gribenko) #18

Yeah, we had those. Try pulling from the branch, I have resolved some
of these issues. I'll be working on HashedCollections next, feel free
to take Flatten and FlatMap.

Dmitri

···

On Wed, Mar 9, 2016 at 3:52 PM, Shawn Erickson <shawnce@gmail.com> wrote:

I see a handful of issues like the following coming about because of changes
to `Range`. In the past `Range` conformed to `Collection` so `count` came
from `Collection` which returned a type of `ForwardIndex.Distance` (e.g.
`_SignedInteger`). It now comes from `Strideable`’s `distance(to:)` which
returns `Strideable.Stride` (e.g. `SignedNumber`).

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr@gmail.com>*/


(Shawn Erickson) #19

Additionally when trying to provide `Strideable` optimized `Collection`

default implementations I hit issues like the following (note
`Collection.IndexDistance` minimally conforms to SignedInteger). I picked
`Strideable` as the optimization point since `RandomAccessCollection.Index`
minimally conforms to `Strideable` and `Strideable` provides the first
level of possible optimizations after `Comparable`.

extension Collection where Index : Strideable {
  public func next(i: Index) -> Index {
    return i.advanced(by: 1) <— this is ok
  }
  public func advance(i: Index, by n: IndexDistance) -> Index {
    return i.advanced(by: n) <— this errors
  }

}

error: cannot invoke 'advanced' with an argument list of type '(by:
Self.IndexDistance)'
    return i.advanced(by: n)
             ^
note: expected an argument list of type '(by: Self.Index.Stride)'
    return i.advanced(by: n)

Actually as soon as I sent that I realized I was looking at the error
backwards... Strideable's advanced(by:) takes something that conforms to
SignedNumber. IndexDistance is a SignedInteger which does conform to
SignedNumber. So am I hitting a limit of the compiler? ...or missing
something obvious here?

···

On Wed, Mar 9, 2016 at 3:52 PM Shawn Erickson <shawnce@gmail.com> wrote:

————————————————————————————————————————
Equatable
  ^
Comparable
  ^
Strideable (associatedtype Stride : SignedNumber) ->
advanced(by:Self.Stride) & distance(to: Self)
  ^
  ^ SignedNumber
  ^ ^
Integer & _SignedInteger —> func toIntMax()
  ^
SignedInteger —> func toIntMax()
————————————————————————————————————————

-Shawn


(Shawn Erickson) #20

Ah I see... should have thought of that fix, forest for the trees.

You also fixed some of the things I had fixed locally! :slight_smile:

Will put aside my changes for all but Flatten at the moment.

Thanks,
-Shawn

···

On Wed, Mar 9, 2016 at 3:59 PM Dmitri Gribenko <gribozavr@gmail.com> wrote:

On Wed, Mar 9, 2016 at 3:52 PM, Shawn Erickson <shawnce@gmail.com> wrote:
> I see a handful of issues like the following coming about because of
changes
> to `Range`. In the past `Range` conformed to `Collection` so `count` came
> from `Collection` which returned a type of `ForwardIndex.Distance` (e.g.
> `_SignedInteger`). It now comes from `Strideable`’s `distance(to:)` which
> returns `Strideable.Stride` (e.g. `SignedNumber`).

Yeah, we had those. Try pulling from the branch, I have resolved some
of these issues. I'll be working on HashedCollections next, feel free
to take Flatten and FlatMap.