[Review] SE-0163 [2]: String Revision: Collection Conformance, C Interop, Transcoding


(John McCall) #1

Hello Swift Community,

The review of SE-0163: "String Revision: Collection Conformance, C Interop, Transcoding" begins now and runs through May 15th, 2017.

The first version of this proposal was accepted with revisions. The proposed revisions were just a few additions that were publicly requested and agreed to during the first review, and this would not normally require a full second review. However, as we worked to implement the proposal, several minor naming changes suggested themselves that had not been previously discussed. The Core Team felt that the most conscientious approach would be to submit the whole proposal for a short re-review.

The most noticeable change is that the Core Team feels StringProtocol would be a better name for the protocol unifying String and Substring. Unicode was felt to be slightly intimidating. Not using Unicode as the protocol name also has the benefit of allowing it to be used as a namespace to house Unicode-related types.
The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0163-string-revision-1.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.

When replying, please try to keep the proposal link at the top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0163-string-revision-1.md
Reply text

Other replies

<https://github.com/apple/swift-evolution#what-goes-into-a-review-1>
What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine 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

Thanks,
John McCall
Review Manager


#2

Looks good at first glance.

Is there a diff showing what’s changed though?

Nevin

···

On Thu, May 11, 2017 at 5:39 PM, John McCall via swift-evolution < swift-evolution@swift.org> wrote:

Hello Swift Community,

The review of SE-0163: "String Revision: Collection Conformance, C
Interop, Transcoding" begins now and runs through May 15th, 2017.

The first version of this proposal was accepted with revisions. The
proposed revisions were just a few additions that were publicly requested
and agreed to during the first review, and this would not normally require
a full second review. However, as we worked to implement the proposal,
several minor naming changes suggested themselves that had not been
previously discussed. The Core Team felt that the most conscientious
approach would be to submit the whole proposal for a short re-review.

The most noticeable change is that the Core Team feels StringProtocol
would be a better name for the protocol unifying String and Substring.
Unicode was felt to be slightly intimidating. Not using Unicode as the
protocol name also has the benefit of allowing it to be used as a namespace
to house Unicode-related types.

The proposal is available here:
https://github.com/apple/swift-evolution/blob/master/
proposals/0163-string-revision-1.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.

When replying, please try to keep the proposal link at the top of the
message:

Proposal link:
https://github.com/apple/swift-evolution/blob/master/
proposals/0163-string-revision-1.md
Reply text

Other replies

<https://github.com/apple/swift-evolution#what-goes-into-a-review-1>

What goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and, eventually, determine 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

Thanks,
John McCall
Review Manager

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


(Brent Royal-Gordon) #3

What is your evaluation of the proposal?

These changes all look like improvements to me. (But I would say that, since I recommended a couple of them in the original review.)

I notice that `Unicode.ParseResult`'s cases have lost their `resumptionPoint`s. Is the intent that the fully-baked versions of the higher-level APIs will pass these out in some other way? If so, is there a possibility that `Unicode.ParseResult` will not be adequate for those types, and it should be given a more specific name like `Unicode.ScalarParseResult`?

(Off-topic, but: What's holding us back from allowing protocols to be nested inside other types? I don't think this is the first design to fake that with typealiases.)

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

Yup.

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

Yes. The change to `StringProtocol` feels particularly nice.

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

Compared to the original version, there's nothing really new that impacts this question.

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

Quick reading of the diffs.

···

On May 11, 2017, at 2:39 PM, John McCall <rjmccall@apple.com> wrote:

--
Brent Royal-Gordon
Architechies


(David Hart) #4

I didn't provide feedback on the first iteration because I felt overwhelmed by all the changes. I think I could better comment on them if I could try them out.

Is there an Xcode toolchain with those changes implemented?

David.

···

On 11 May 2017, at 23:39, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift Community,

The review of SE-0163: "String Revision: Collection Conformance, C Interop, Transcoding" begins now and runs through May 15th, 2017.

The first version of this proposal was accepted with revisions. The proposed revisions were just a few additions that were publicly requested and agreed to during the first review, and this would not normally require a full second review. However, as we worked to implement the proposal, several minor naming changes suggested themselves that had not been previously discussed. The Core Team felt that the most conscientious approach would be to submit the whole proposal for a short re-review.

The most noticeable change is that the Core Team feels StringProtocol would be a better name for the protocol unifying String and Substring. Unicode was felt to be slightly intimidating. Not using Unicode as the protocol name also has the benefit of allowing it to be used as a namespace to house Unicode-related types.
The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0163-string-revision-1.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.

When replying, please try to keep the proposal link at the top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0163-string-revision-1.md
Reply text

Other replies

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine 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

Thanks,
John McCall
Review Manager
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(John McCall) #5

Looks good at first glance.

Is there a diff showing what’s changed though?

This seems to work:

  https://github.com/apple/swift-evolution/compare/4cbb1f1fa836496d4bfba95c4b78a9754690956d...master#diff-e04958ae62d5b65cb356dfe928b24837

(Ignore the changes to other files.)

John.

···

On May 11, 2017, at 6:08 PM, Nevin Brackett-Rozinsky via swift-evolution <swift-evolution@swift.org> wrote:

Nevin

On Thu, May 11, 2017 at 5:39 PM, John McCall via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
Hello Swift Community,

The review of SE-0163: "String Revision: Collection Conformance, C Interop, Transcoding" begins now and runs through May 15th, 2017.

The first version of this proposal was accepted with revisions. The proposed revisions were just a few additions that were publicly requested and agreed to during the first review, and this would not normally require a full second review. However, as we worked to implement the proposal, several minor naming changes suggested themselves that had not been previously discussed. The Core Team felt that the most conscientious approach would be to submit the whole proposal for a short re-review.

The most noticeable change is that the Core Team feels StringProtocol would be a better name for the protocol unifying String and Substring. Unicode was felt to be slightly intimidating. Not using Unicode as the protocol name also has the benefit of allowing it to be used as a namespace to house Unicode-related types.
The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0163-string-revision-1.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.

When replying, please try to keep the proposal link at the top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0163-string-revision-1.md
Reply text

Other replies

<https://github.com/apple/swift-evolution#what-goes-into-a-review-1>
What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine 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

Thanks,
John McCall
Review Manager

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

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


(Dave Abrahams) #6

What is your evaluation of the proposal?

These changes all look like improvements to me. (But I would say that, since I recommended a couple
of them in the original review.)

I notice that `Unicode.ParseResult`'s cases have lost their
`resumptionPoint`s. Is the intent that the fully-baked versions of the
higher-level APIs will pass these out in some other way?

Sort of. The latest updates have a “length” in the error case and a
“payload.count” in the valid case, which should give you the information
you need to derive a new index. The problem we had with vending Index
resumption points was that it penalized use with Sequences, which don't
have indices. Even though parsing/decoding Sequences that aren't also
Collections is probably a marginal use-case, that pattern was wound into
the code paths of our existing implementations and our microbenchmarks
have been tuned such that they detect any degradation of performance in
that use-case.

If so, is there a possibility that `Unicode.ParseResult` will not be
adequate for those types, and it should be given a more specific name
like `Unicode.ScalarParseResult`?

It's actually a very generally-useful parse result type AFAICT; we just
stuck it in the Unicode namespace to avoid being presumptuous about what
would be needed for other applications. I'm betting the same exact
structure will work for general pattern matching, for example.

···

on Thu May 11 2017, Brent Royal-Gordon <swift-evolution@swift.org> wrote:

On May 11, 2017, at 2:39 PM, John McCall <rjmccall@apple.com> wrote:

--
-Dave


(TJ Usiyan) #7

+1 overall.

Is there any hope for a diagnostic to call violations of the issue outlined
in the following passage out?

any passing of self into an API that takes a concrete String will need to

be rewritten as String(self). If Self is a String then this should
effectively optimize to a no-op, whereas if Self is a Substring then this
will force a copy, helping to avoid the "memory leak" problems described
above.

"a closure expression which is immediately caled,"

···

On Thu, May 11, 2017 at 6:32 PM, John McCall via swift-evolution < swift-evolution@swift.org> wrote:

On May 11, 2017, at 6:08 PM, Nevin Brackett-Rozinsky via swift-evolution < > swift-evolution@swift.org> wrote:

Looks good at first glance.

Is there a diff showing what’s changed though?

This seems to work:

  https://github.com/apple/swift-evolution/compare/
4cbb1f1fa836496d4bfba95c4b78a9754690956d...master#diff-
e04958ae62d5b65cb356dfe928b24837

(Ignore the changes to other files.)

John.

Nevin

On Thu, May 11, 2017 at 5:39 PM, John McCall via swift-evolution < > swift-evolution@swift.org> wrote:

Hello Swift Community,

The review of SE-0163: "String Revision: Collection Conformance, C
Interop, Transcoding" begins now and runs through May 15th, 2017.

The first version of this proposal was accepted with revisions. The
proposed revisions were just a few additions that were publicly requested
and agreed to during the first review, and this would not normally require
a full second review. However, as we worked to implement the proposal,
several minor naming changes suggested themselves that had not been
previously discussed. The Core Team felt that the most conscientious
approach would be to submit the whole proposal for a short re-review.

The most noticeable change is that the Core Team feels StringProtocol
would be a better name for the protocol unifying String and Substring.
Unicode was felt to be slightly intimidating. Not using Unicode as the
protocol name also has the benefit of allowing it to be used as a namespace
to house Unicode-related types.

The proposal is available here:
https://github.com/apple/swift-evolution/blob/master/proposa
ls/0163-string-revision-1.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.

When replying, please try to keep the proposal link at the top of the
message:

Proposal link:
https://github.com/apple/swift-evolution/blob/master/proposa
ls/0163-string-revision-1.md
Reply text

Other replies

<https://github.com/apple/swift-evolution#what-goes-into-a-review-1>

What goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and, eventually, determine 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

Thanks,
John McCall
Review Manager

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

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

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


(John McCall) #8

"a closure expression which is immediately caled,"

Thanks, fixed. (To be clear, that proposal is not part of SE-0163.)

John.

···

On May 11, 2017, at 8:47 PM, T.J. Usiyan <griotspeak@gmail.com> wrote:

On Thu, May 11, 2017 at 6:32 PM, John McCall via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On May 11, 2017, at 6:08 PM, Nevin Brackett-Rozinsky via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

Looks good at first glance.

Is there a diff showing what’s changed though?

This seems to work:

  https://github.com/apple/swift-evolution/compare/4cbb1f1fa836496d4bfba95c4b78a9754690956d...master#diff-e04958ae62d5b65cb356dfe928b24837

(Ignore the changes to other files.)

John.

Nevin

On Thu, May 11, 2017 at 5:39 PM, John McCall via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
Hello Swift Community,

The review of SE-0163: "String Revision: Collection Conformance, C Interop, Transcoding" begins now and runs through May 15th, 2017.

The first version of this proposal was accepted with revisions. The proposed revisions were just a few additions that were publicly requested and agreed to during the first review, and this would not normally require a full second review. However, as we worked to implement the proposal, several minor naming changes suggested themselves that had not been previously discussed. The Core Team felt that the most conscientious approach would be to submit the whole proposal for a short re-review.

The most noticeable change is that the Core Team feels StringProtocol would be a better name for the protocol unifying String and Substring. Unicode was felt to be slightly intimidating. Not using Unicode as the protocol name also has the benefit of allowing it to be used as a namespace to house Unicode-related types.
The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0163-string-revision-1.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.

When replying, please try to keep the proposal link at the top of the message:

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0163-string-revision-1.md
Reply text

Other replies

<https://github.com/apple/swift-evolution#what-goes-into-a-review-1>
What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine 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

Thanks,
John McCall
Review Manager

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

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

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