[Review] SE-0093: Adding a public base property to slices


(Dave Abrahams) #1

Hello Swift community,

The review of "SE-0093: Adding a public base property to slices"
begins now and runs through May 23. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0093-slice-base.md

Reviews are an important part of the Swift evolution process. All
reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and contribute to the direction of
Swift. When writing your review, here are some questions you might
want to answer in your review:

  * What is your evaluation of the proposal?

  * Is the problem being addressed significant enough to warrant a
          change to Swift?

  * Does this proposal fit well with the feel and direction of
          Swift?

  * If you have used other languages or libraries with a similar
          feature, how do you feel that this proposal compares to those?

  * How much effort did you put into your review? A glance, a
          quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

Dave Abrahams
Review Manager


(Lily Ballard) #2

  * What is your evaluation of the proposal?

The motivation sounds reasonable, as does the solution. But it seems odd to expose a property `base` on MutableRandomAccessSlice without exposing it on any other slice type. I'd much rather expose it everywhere, ideally by renaming the `_base` property as suggested in the alternatives section. Stdlib breakage can be handled on a temporary basis by providing the `_base` accessor as a computed property that returns `base`, though of course the goal should be to remove this entirely (or hopefully not have it at all if there's not too much stdlib breakage). And such a change should still be purely additive from the perspective of third-party code.

  * Is the problem being addressed significant enough to warrant a
          change to Swift?

Yes. This is a relatively minor change but it allows for better performance.

  * Does this proposal fit well with the feel and direction of
          Swift?

Yes.

  * If you have used other languages or libraries with a similar
          feature, how do you feel that this proposal compares to those?

I can't think of any languages with this offhand.

  * How much effort did you put into your review? A glance, a
          quick reading, or an in-depth study?

A quick reading.

-Kevin Ballard

···

On Thu, May 19, 2016, at 04:43 PM, Dave Abrahams via swift-evolution wrote:


(Jordan Rose) #3

[Proposal: https://github.com/apple/swift-evolution/blob/master/proposals/0093-slice-base.md <https://github.com/apple/swift-evolution/blob/master/proposals/0093-slice-base.md]>]

Hey, Max. For clarification, this isn’t adding anything to the requirements for slice types, correct? That is, the result type of 'subscript(_: Range)' has no additional requirements? (Why I ask: there would be plenty of implementations of slices that may not otherwise need a reference to the original collection.)

I’m vaguely uncomfortable with this because I feel like it’s violating some abstraction or allowing unsafe indexing, but of course that’s no more unsafe than with any other collection.

Jordan

···

On May 19, 2016, at 16:43, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0093: Adding a public base property to slices"
begins now and runs through May 23. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0093-slice-base.md

Reviews are an important part of the Swift evolution process. All
reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and contribute to the direction of
Swift. When writing your review, here are some questions you might
want to answer in your review:

  * What is your evaluation of the proposal?

  * Is the problem being addressed significant enough to warrant a
         change to Swift?

  * Does this proposal fit well with the feel and direction of
         Swift?

  * If you have used other languages or libraries with a similar
         feature, how do you feel that this proposal compares to those?

  * How much effort did you put into your review? A glance, a
         quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

Dave Abrahams
Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Dave Abrahams) #4

Proposal Link: https://github.com/apple/swift-evolution/blob/master/proposals/0093-slice-base.md

The review of "SE-0093: Adding a public base property to slices" ran from May 19…23, 2016. The
proposal has been *accepted* without changes:

- There was little feedback from the community other than to get
  clarification.
- The core team recognizes the proposal as exposing an important basis
  operation of slice types.
  
Thank you to Max Moiseev for proposing it!

-Dave Abrahams
Review Manager


(Max Moiseev) #5

Hi Kevin,

Thanks for reviewing this proposal.

It is my poor choice of words to be blamed for the confusion. There is definitely no reason for the new property to only be available for the MutableRandomAccessSlice type. Moreover, since all the slice types are generated with GYB now, it would be more code :wink:

The intention is to add the new property to ALL the slice types.

Regards,
Max

···

On May 23, 2016, at 3:08 PM, Kevin Ballard via swift-evolution <swift-evolution@swift.org> wrote:

On Thu, May 19, 2016, at 04:43 PM, Dave Abrahams via swift-evolution wrote:

  * What is your evaluation of the proposal?

The motivation sounds reasonable, as does the solution. But it seems odd to expose a property `base` on MutableRandomAccessSlice without exposing it on any other slice type. I'd much rather expose it everywhere, ideally by renaming the `_base` property as suggested in the alternatives section. Stdlib breakage can be handled on a temporary basis by providing the `_base` accessor as a computed property that returns `base`, though of course the goal should be to remove this entirely (or hopefully not have it at all if there's not too much stdlib breakage). And such a change should still be purely additive from the perspective of third-party code.

  * Is the problem being addressed significant enough to warrant a
         change to Swift?

Yes. This is a relatively minor change but it allows for better performance.

  * Does this proposal fit well with the feel and direction of
         Swift?

Yes.

  * If you have used other languages or libraries with a similar
         feature, how do you feel that this proposal compares to those?

I can't think of any languages with this offhand.

  * How much effort did you put into your review? A glance, a
         quick reading, or an in-depth study?

A quick reading.

-Kevin Ballard
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Max Moiseev) #6

That is correct, the proposed change only applies to the concrete slice types that are provided by the standard library.

Max

···

On May 23, 2016, at 5:13 PM, Jordan Rose <jordan_rose@apple.com> wrote:

[Proposal: https://github.com/apple/swift-evolution/blob/master/proposals/0093-slice-base.md <https://github.com/apple/swift-evolution/blob/master/proposals/0093-slice-base.md]>]

Hey, Max. For clarification, this isn’t adding anything to the requirements for slice types, correct? That is, the result type of 'subscript(_: Range)' has no additional requirements? (Why I ask: there would be plenty of implementations of slices that may not otherwise need a reference to the original collection.)

I’m vaguely uncomfortable with this because I feel like it’s violating some abstraction or allowing unsafe indexing, but of course that’s no more unsafe than with any other collection.

Jordan

On May 19, 2016, at 16:43, Dave Abrahams via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Hello Swift community,

The review of "SE-0093: Adding a public base property to slices"
begins now and runs through May 23. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0093-slice-base.md

Reviews are an important part of the Swift evolution process. All
reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and contribute to the direction of
Swift. When writing your review, here are some questions you might
want to answer in your review:

  * What is your evaluation of the proposal?

  * Is the problem being addressed significant enough to warrant a
         change to Swift?

  * Does this proposal fit well with the feel and direction of
         Swift?

  * If you have used other languages or libraries with a similar
         feature, how do you feel that this proposal compares to those?

  * How much effort did you put into your review? A glance, a
         quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

Dave Abrahams
Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Chris Lattner) #7

FYI.

···

Begin forwarded message:

From: Dave Abrahams via swift-evolution <swift-evolution@swift.org>
Date: May 26, 2016 at 12:42:53 PM PDT
To: swift-evolution@swift.org
Subject: [swift-evolution] [Accepted] SE-0093: Adding a public base property to slices

Proposal Link: https://github.com/apple/swift-evolution/blob/master/proposals/0093-slice-base.md

The review of "SE-0093: Adding a public base property to slices" ran from May 19…23, 2016. The
proposal has been *accepted* without changes:

- There was little feedback from the community other than to get
clarification.
- The core team recognizes the proposal as exposing an important basis
operation of slice types.

Thank you to Max Moiseev for proposing it!

-Dave Abrahams
Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Lily Ballard) #8

Hi Kevin,

Thanks for reviewing this proposal.

It is my poor choice of words to be blamed for the confusion. There is definitely no reason for the new property to only be available for the MutableRandomAccessSlice type. Moreover, since all the slice types are generated with GYB now, it would be more code :wink:

The intention is to add the new property to ALL the slice types.

Ah hah, you're right, I missed the following line when reading:

The same change is applicable to both mutable and immutable slice types.

So +1, though I'm still in favor of renaming `_base` to `base` and then adding a computed `_base` property back if the stdlib breakage is too high.

-Kevin Ballard

···

On Mon, May 23, 2016, at 03:15 PM, Max Moiseev wrote: