[Review] SE-0027 Expose code unit initializers on String


(Douglas Gregor) #1

Hello Swift community,

The review of SE-0027 "Expose code unit initializers on String" begins now and runs through February 16, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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/0027-string-from-code-units.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
Thank you,

Doug Gregor

Review Manager


(Tony Parker) #2

Hi Doug, Zachary,

Can you provide more clarification on why the static func is needed? It seems like the functionality is all about initialization and therefore belongs in an initializer (as is also proposed).

- Tony

···

On Feb 11, 2016, at 4:41 PM, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0027 "Expose code unit initializers on String" begins now and runs through February 16, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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/0027-string-from-code-units.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
Thank you,

Doug Gregor

Review Manager

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


(Marco Masser) #3

+1

• What is your evaluation of the proposal?

As the proposal points out, converting Strings to/from their byte representations is common and not at all obvious or easy in Swift at the moment. IMHO, the best solution at the moment is to revert to NSString initializers that take an UnsafePointer<Void>, a length and an encoding.

• 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?

It’s definitely a good addition to the standard library. Being independent from the NSString Objective-C implementation for basic things is a good idea.

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

It’s a Swifty set of APIs, so I’d say the proposal is fine.
I don’t have a concrete use for the proposed static decode() function that returns whether invalid code units were repaired. But I don’t think there’s much harm in keeping that if some client is interested in it.

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

I was confronted with the exact problem that is being addressed the last couple of days that consisted of the following steps:
• Write a small protocol oriented wrapper around the POSIX socket API that offers methods for sending and receiving Strings.
• Try to convert Strings to/from a collection of UTF8 bytes.
• Read Swift’s standard library source code to see how it’s handled there.
• Revert to NSString bridging.
• Mentally prepare to write proposal to expose these APIs.
• Be delighted that there already is a proposal that is 100% what I wanted.


(Brent Royal-Gordon) #4

https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.md

Might I suggest that you include doc comments with each of your proposed member in the proposal? I support the idea in general, but it's hard to understand the exact semantics of each proposed member if you don't already know how String's internals work.

···

--
Brent Royal-Gordon
Architechies


(Patrick Gili) #5

Hello Swift community,

The review of SE-0027 "Expose code unit initializers on String" begins now and runs through February 16, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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/0027-string-from-code-units.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?

In general, I support the proposal. However, I have a problem treating String as a bucket of bytes. The introduction starts out by making the claim, "Going back and forth from Strings to their byte representations is an important part of solving many problems, including object serialization, binary and text file formats, wire/network interfaces, and cryptography." Essentially, these problems deal with an array of raw bytes, and I have to wonder why an application would push them into a String? This is the old problem we had in C with treating char[] as a an array of raw bytes, which we introduced numerous problems over time, especially when new character encodings came into the picture. We learned the hard way that we had to declare byte arrays as uint8[].

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

If an application developer very much wants to leverage the methods made available by String, I suppose this addresses a problem. However, I prefer the idea of adding the necessary view(s) to String, as was mentioned in the alternatives section of the proposal. Would String.UTF8View provide the same capabilities as String.UInt8View?

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

I don't feel like it does. I like the alternative of new views better. String already has the CharacterView, and SE-0010 (https://github.com/apple/swift-evolution/blob/master/proposals/0010-add-staticstring-unicodescalarview.md) introduced the UnicodeScalarView (although, I don't know if this has been approved). If Swift is going to promote this notion of "views", then I like this alternative even more.

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

Safely? I can't think of an example.

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

Went back and reread through the thread that drove this proposal. Thoroughly read the proposal. Gave the implementation a quick look.

···

On Feb 11, 2016, at 7:41 PM, Douglas Gregor <dgregor@apple.com> wrote:

More information about the Swift evolution process is available at

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

Doug Gregor

Review Manager

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


(David Hart) #6

What is your evaluation of the proposal?
+1. I’ve never had to work with string byte representations but the proposal makes sense.

Is the problem being addressed significant enough to warrant a change to Swift?
It changes the standard library instead of the language, so I think it is warranted enough for that.

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

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
I have used the byte APIs on NSString in Objective-C. Sounds like it will bring Swift’s String up-par.

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

···

On 12 Feb 2016, at 01:41, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0027 "Expose code unit initializers on String" begins now and runs through February 16, 2016. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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/0027-string-from-code-units.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
Thank you,

Doug Gregor

Review Manager

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


(Dave Abrahams) #7

Hello Swift community,

The review of SE-0027 "Expose code unit initializers on String" begins
now and runs through February 16, 2016. The proposal is available
here:

https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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
<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/0027-string-from-code-units.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?

I support the intent of the proposal but am -1 on the specific proposal
as offered.

First, in the presentation, there are several things that make it hard
to evaluate:

1. The "decode" function signature is so wide that it can't be read
   without scrolling at
   https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.md

2. The proposal doesn't show any usage of the proposed APIs, so it
   is hard to understand what effect these APIs would have on real
   code. There are some examples of uses in
   https://github.com/apple/swift/compare/master...zwaldowski:string-from-code-units
   but I have to dig for them.

3. The description of the “Detailed Design” doesn't show any examples
   either, so I have to imagine what “backporting the Swift 3.0 versions
   of the CString constructors, then making them generic over their
   input and codec” means.

Next, when I look at *uses* that I can find (in the tests), I don't find
them to be clear.

  String(validatingCodeUnits: result, as: UTF8.self)

What does “validatingCodeUnits” mean? Clearly we're going to do some
checking. Is there a repair? Is the initializer failable?

In this change, for example:
https://github.com/apple/swift/compare/master...zwaldowski:string-from-code-units#diff-d39ec2c4819c950aeef95f899f53b104R79
I find the old code much clearer than the new code (I'm not sure why
_decode starts with an underscore here; isn't it part of your proposed
public API?)

Lastly, I am very concerned about the “Alternatives Considered” section,
where, of one alternative, it says:

      This might be the better long-term solution from the perspective
      of API maintenance, but in the meantime this proposal has a fairly
      low impact.

We can't accept changes into the standard library “in the meantime,”
with the expectation that something more comprehensive will make them
obsolete. Even though we've had migration tools, we never operated that
way in the past, and as we head toward API and ABI stability it is even
more true today.

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

Yes.

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

W.r.t. direction, the fact that we have a major String overhaul planned
means that tackling this one corner of the API is probably not entirely
appropriate.

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

between quick reading and in-depth study.

···

on Thu Feb 11 2016, Douglas Gregor <swift-evolution@swift.org> wrote:

--
-Dave


(Charles Kissinger) #8

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.md
What is your evaluation of the proposal?

I strongly support this proposal. It fills an important gap in the String API, and it would be of immediate benefit to me.

Although the performance benefits of the proposed initializers are listed as a primary motivation for the proposal, I think it is important to add them for completeness of the String API as well. UTF code unit sequences are the standard, cross-platform serialization formats for Unicode text, and the native Swift String type should have full support for interconversion to and from UTF.

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

Yes. This is a surprising omission from the API and a significant pain point for those of us doing certain kinds of parsing.

I would prefer that one additional String method be added that would allow code unit sequences to be directly *appended* to Strings:

String.appendContentsOf<S : SequenceType, Encoding: UnicodeCodecType where Encoding.CodeUnit == Input.Generator.Element>(_: S, encoding: Encoding.Type)

A brief discussion of this additional method, and the motivation for adding it, is in the later parts of the original discussion thread for this proposal.

My understanding is that a simple implementation of this method would not have immediate performance benefits as will be the case for the new initializers. Some reworking of the internals of String would be necessary for that. I think it should be added for the sake of API completeness though, with optimal performance perhaps coming somewhere down the road.

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

Yes. It is a great improvement over forcing people to use intermediate null-terminated “C” strings to efficiently initialize a String from code units!

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

N/A

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

I participated in the mailing list discussion of this proposal, made suggestions for method name improvements, provided feedback on the text of the proposal, and looked through the existing and proposed String initialization code.

I’ve closely examined the performance characteristics of various methods of String/code-unit interconversion during the course of my work.


(Lily Ballard) #9

I'm very much a fan of the end result of this proposal, i.e. some way to
convert from a collection of code units into a String. Initially I
thought the proposed API was acceptable, barring some quibbles about
naming, but I think Dave Abrahams has made some good points in this
thread, and so I'll just say that I very much support getting this kind
of functionality in the stdlib, but if Dave Abrahams thinks that the API
as proposed isn't what we want in the long term, then I'll defer to his
judgement.

Also, given the proposed API, the input really should be a SequenceType.
I recognize that it's calling through to _StringBuffer.fromCodeUnits()
which takes a CollectionType, but the only reason that takes a
CollectionType is so it can pre-calculate the size of the UTF-16 buffer,
and I don't think that limitation should be exposed through the public
API. It would be better to change _StringBuffer.fromCodeUnits() to take
a SequenceType and use the _preprocessingPass machinery to choose
between the fast-path of pre-allocating the buffer and a slow path of
growing the buffer as needed.

-Kevin Ballard

···

On Thu, Feb 11, 2016, at 04:41 PM, Douglas Gregor wrote:

Hello Swift community,

The review of SE-0027 "Expose code unit initializers on String"
begins now and runs through February 16, 2016. The proposal is
available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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/0027-string-from-code-units.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

Thank you,

Doug Gregor

Review Manager

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


(Zachary Waldowski) #10

It arose out of the original motivation to expose and maintain the
existing methods. Users might find the `hadError` flag to be useful when
code sequence repairs are enabled. It seems like that was the original
intent, too, but everyone in the stdlib is throwing that away.

Looking at it now, I don't think it's needed as public API. I'm in the
midst of rebasing the branch onto master, so I'll play with it on there
and update the proposal to fit.

From an API surface area perspective, the legacy CString constructors

need the full static (not necessarily public then, of course). In a
perfect world, once those are gone, the static can get elided into the
two initializers… which makes the case even more solid for not exporting
it in this proposal.

Cheers! Zachary Waldowski zach@waldowski.me

···

On Thu, Feb 11, 2016, at 09:15 PM, Tony Parker via swift-evolution wrote:

Hi Doug, Zachary,

Can you provide more clarification on why the static func is needed?
It seems like the functionality is all about initialization and
therefore belongs in an initializer (as is also proposed).

- Tony

On Feb 11, 2016, at 4:41 PM, Douglas Gregor via swift-evolution <swift- >> evolution@swift.org> wrote:

Hello Swift community,

The review of SE-0027 "Expose code unit initializers on String"
begins now and runs through February 16, 2016. The proposal is
available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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/0027-string-from-code-units.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

Thank you,

Doug Gregor

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


(Zachary Waldowski) #11

Apologies, these have been written and are included in the PR to Swift:
https://github.com/apple/swift/compare/master...zwaldowski:string-from-code-units#diff-7718fce81316e0b10115494af682cc23R139

I can revise the proposal to include them.

Thanks!

···

On Fri, Feb 12, 2016, at 04:17 PM, Brent Royal-Gordon via swift-evolution wrote:

> https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.md

Might I suggest that you include doc comments with each of your proposed
member in the proposal? I support the idea in general, but it's hard to
understand the exact semantics of each proposed member if you don't
already know how String's internals work.

--
Brent Royal-Gordon
Architechies

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


(Brent Royal-Gordon) #12

The introduction starts out by making the claim, "Going back and forth from Strings to their byte representations is an important part of solving many problems, including object serialization, binary and text file formats, wire/network interfaces, and cryptography." Essentially, these problems deal with an array of raw bytes, and I have to wonder why an application would push them into a String?

I read this section as trying to say "object serialization, binary and text file formats, wire/network interfaces, and cryptography all require you to construct strings from decoded bytes, which is what this proposal is trying to improve". I don't think it's trying to say that we should have better support for treating strings as bags of arbitrary bytes, and in fact I don't think this proposal does that.

···

--
Brent Royal-Gordon
Architechies


(Patrick Gili) #13

Okay. However, does this change the implied semantics?

···

On Feb 13, 2016, at 5:26 PM, Brent Royal-Gordon <brent@architechies.com> wrote:

The introduction starts out by making the claim, "Going back and forth from Strings to their byte representations is an important part of solving many problems, including object serialization, binary and text file formats, wire/network interfaces, and cryptography." Essentially, these problems deal with an array of raw bytes, and I have to wonder why an application would push them into a String?

I read this section as trying to say "object serialization, binary and text file formats, wire/network interfaces, and cryptography all require you to construct strings from decoded bytes, which is what this proposal is trying to improve". I don't think it's trying to say that we should have better support for treating strings as bags of arbitrary bytes, and in fact I don't think this proposal does that.

--
Brent Royal-Gordon
Architechies


(Zachary Waldowski) #14

See responses inline.

Aside: omgomgomg I got Dave Abrahams to respond to a proposal of mine

Excitedly,
Zachary Waldowski
zach@waldowski.me

> Hello Swift community,
>
> The review of SE-0027 "Expose code unit initializers on String" begins
> now and runs through February 16, 2016. The proposal is available
> here:
>
> https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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
> <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/0027-string-from-code-units.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?

I support the intent of the proposal but am -1 on the specific proposal
as offered.

First, in the presentation, there are several things that make it hard
to evaluate:

1. The "decode" function signature is so wide that it can't be read
   without scrolling at
   https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.md

2. The proposal doesn't show any usage of the proposed APIs, so it
   is hard to understand what effect these APIs would have on real
   code. There are some examples of uses in
   https://github.com/apple/swift/compare/master...zwaldowski:string-from-code-units
   but I have to dig for them.

3. The description of the “Detailed Design” doesn't show any examples
   either, so I have to imagine what “backporting the Swift 3.0 versions
   of the CString constructors, then making them generic over their
   input and codec” means.

The notes are appreciated. I'll take care to add examples and fix
formatting. The proposal was always intended to be paired with the PR to
the stdlib, and that lack of clarity did not come up during the proposal
vetting stage.

Next, when I look at *uses* that I can find (in the tests), I don't find
them to be clear.

  String(validatingCodeUnits: result, as: UTF8.self)

What does “validatingCodeUnits” mean? Clearly we're going to do some
checking. Is there a repair? Is the initializer failable?

I'm open to suggestions. The names as they are were lifted from the
changes to the CString APIs made on swift-3-api-guidelines.

"repairingCodeUnits" implies to me that there is something already wrong
with them. I'm also not sure as to the point of the initializer being
failable; many domain initializers in the stdlib don't indicate that
they can fail directly in the name.

In this change, for example:
https://github.com/apple/swift/compare/master...zwaldowski:string-from-code-units#diff-d39ec2c4819c950aeef95f899f53b104R79
I find the old code much clearer than the new code (I'm not sure why
_decode starts with an underscore here; isn't it part of your proposed
public API?)

The PR is ahead of the proposal here. It was pointed out in the review
thread up to this point that that that "primitive" isn't truly needed;
the initializers on String should be the API used by all.

The ugliness in this instance is transitional; it stems from bridging
the old-style constructor to the new-style one. The old signature's
return type is awkward in that it returns the extra hadError flag
regardless of whether decoding failed, which is discarded almost
universally by clients in the stdlib.

I intend to revert the change to `fromCStringRepairingIllFormedUTF8`, or
elide `_decode` into it instead.

Lastly, I am very concerned about the “Alternatives Considered” section,
where, of one alternative, it says:

      This might be the better long-term solution from the perspective
      of API maintenance, but in the meantime this proposal has a fairly
      low impact.

We can't accept changes into the standard library “in the meantime,”
with the expectation that something more comprehensive will make them
obsolete. Even though we've had migration tools, we never operated that
way in the past, and as we head toward API and ABI stability it is even
more true today.

I understand the sentiment here, but I don't think "wait for the next
rewrite of String" is a good solution to a problem that the stdlib
already resolves. See below.

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

Yes.

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

W.r.t. direction, the fact that we have a major String overhaul planned
means that tackling this one corner of the API is probably not entirely
appropriate.

"Corner" is an unfair characterization; users of Swift want to implement
things to the level of safety and performance bar set by the stdlib, but
we are currently at a disadvantage. No better example can be found but
in corelibs-foundation:
https://github.com/apple/swift-corelibs-foundation/blob/546dc8e16c3c34ca50f5752c6d0f39c3524f5f0a/Foundation/NSString.swift#L305.
Is it not a deficiency of the stdlib when code has to resort to
non-public methods? The stdlib (i.e., the parts touched by the PR) and
corelibs-foundation would have to move in lockstep to adopt a
replacement, so the existing underscored versions are as good as public
API.

I understand, and 100% encourage, the reticence around new API. However,
we've created a worse problem by encouraging slow, buggy, custom
versions of behavior that already exists in the stdlib, or implying that
the underscored API should be used because we haven't managed something
better yet. :confused:

···

On Mon, Feb 15, 2016, at 11:48 AM, Dave Abrahams via swift-evolution wrote:

on Thu Feb 11 2016, Douglas Gregor <swift-evolution@swift.org> wrote:

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

between quick reading and in-depth study.

--
-Dave

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


(Dave Abrahams) #15

I'm very much a fan of the end result of this proposal, i.e. some way to
convert from a collection of code units into a String. Initially I
thought the proposed API was acceptable, barring some quibbles about
naming, but I think Dave Abrahams has made some good points in this
thread, and so I'll just say that I very much support getting this kind
of functionality in the stdlib, but if Dave Abrahams thinks that the API
as proposed isn't what we want in the long term, then I'll defer to his
judgement.

I should be clear: I based that assessment on what the proposal itself
said. I don't even know the details of the alternative that the
proposal suggests would be better in the long term. The fact that the
proposal is suggesting we do something that it says might not be the
best long-term solution, with no other justification than that it will
work “in the meantime” leaves me lacking confidence that the proposal
is sufficiently well-considered to accept.

···

on Tue Feb 16 2016, Kevin Ballard <swift-evolution@swift.org> wrote:

Also, given the proposed API, the input really should be a SequenceType.
I recognize that it's calling through to _StringBuffer.fromCodeUnits()
which takes a CollectionType, but the only reason that takes a
CollectionType is so it can pre-calculate the size of the UTF-16 buffer,
and I don't think that limitation should be exposed through the public
API. It would be better to change _StringBuffer.fromCodeUnits() to take
a SequenceType and use the _preprocessingPass machinery to choose
between the fast-path of pre-allocating the buffer and a slow path of
growing the buffer as needed.

-Kevin Ballard

On Thu, Feb 11, 2016, at 04:41 PM, Douglas Gregor wrote:

Hello Swift community,

The review of SE-0027 "Expose code unit initializers on String"
begins now and runs through February 16, 2016. The proposal is
available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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/0027-string-from-code-units.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

Thank you,

Doug Gregor

Review Manager

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

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

--
-Dave


(Zachary Waldowski) #16

I think you're drawing an overly arbitrary distinction about the
semantics. I'd recommend a close re-reading of the Swift book's chapters
on String after their reworking in 2.0; it bridges together the "linked
list of text-ish things", "collection of Characters", and "bag of bytes"
ideas rather well. They're not mutually exclusive.

The new methods do not decrease the safety of String, nor does it change
the contract of the API design. It should not be possible to get
malformed strings back from the new API; the non-validating version
automatically performs repairs, and the validating version fails (by
returning nil) on any errors. In fact, exposing these APIs in a way that
is aware and respectful of String's underpinnings is safer than the
alternative. The stdlib won't screw up things like surrogate pairs or
range checking of valid code points, whereas I've seen plenty of code
try and do what these methods do themselves by upcasting UInt8 to
UnicodeScalar and accumulating.

Addressing other points about the proposal: I overall agree with you
that the Views would do a better job of this on the long scale of time,
but C and ObjC interop simple require entry points like the ones in this
proposal, and are in-line with how Swift works today. This proposal is
not intended to overhaul String, even though that may be one day
desirable by what Dave and others said on the Evolution thread.

Thanks for your feedback! :slight_smile:

Zach Waldowski
zach@waldowski.me

···

On Sat, Feb 13, 2016, at 05:33 PM, Patrick Gili via swift-evolution wrote:

Okay. However, does this change the implied semantics?

> On Feb 13, 2016, at 5:26 PM, Brent Royal-Gordon <brent@architechies.com> wrote:
>
>> The introduction starts out by making the claim, "Going back and forth from Strings to their byte representations is an important part of solving many problems, including object serialization, binary and text file formats, wire/network interfaces, and cryptography." Essentially, these problems deal with an array of raw bytes, and I have to wonder why an application would push them into a String?
>
> I read this section as trying to say "object serialization, binary and text file formats, wire/network interfaces, and cryptography all require you to construct strings from decoded bytes, which is what this proposal is trying to improve". I don't think it's trying to say that we should have better support for treating strings as bags of arbitrary bytes, and in fact I don't think this proposal does that.
>
> --
> Brent Royal-Gordon
> Architechies
>

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


(Dave Abrahams) #17

See responses inline.

Aside: omgomgomg I got Dave Abrahams to respond to a proposal of mine

Don't worry; it'll wear off soon enough...

Excitedly,
Zachary Waldowski
zach@waldowski.me

> Hello Swift community,
>
> The review of SE-0027 "Expose code unit initializers on String" begins
> now and runs through February 16, 2016. The proposal is available
> here:
>
> https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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
> <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/0027-string-from-code-units.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?

I support the intent of the proposal but am -1 on the specific proposal
as offered.

First, in the presentation, there are several things that make it hard
to evaluate:

1. The "decode" function signature is so wide that it can't be read
   without scrolling at
   https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.md

2. The proposal doesn't show any usage of the proposed APIs, so it
   is hard to understand what effect these APIs would have on real
   code. There are some examples of uses in
   https://github.com/apple/swift/compare/master...zwaldowski:string-from-code-units
   but I have to dig for them.

3. The description of the “Detailed Design” doesn't show any examples
   either, so I have to imagine what “backporting the Swift 3.0 versions
   of the CString constructors, then making them generic over their
   input and codec” means.

The notes are appreciated. I'll take care to add examples and fix
formatting. The proposal was always intended to be paired with the PR to
the stdlib, and that lack of clarity did not come up during the proposal
vetting stage.

Next, when I look at *uses* that I can find (in the tests), I don't find
them to be clear.

  String(validatingCodeUnits: result, as: UTF8.self)

What does “validatingCodeUnits” mean? Clearly we're going to do some
checking. Is there a repair? Is the initializer failable?

I'm open to suggestions. The names as they are were lifted from the
changes to the CString APIs made on swift-3-api-guidelines.

I suspected that they might be so I went and checked on GitHub... but
now I see that only searches the default branch :stuck_out_tongue:

    https://help.github.com/articles/searching-code/

But if the naming error was on our side, my apologies. We should do
better, regardless.

"repairingCodeUnits" implies to me that there is something already wrong
with them.

That doesn't bother me. If you are requesting the repair, you are
effectively assuming that there is something already wrong with them.

I'm also not sure as to the point of the initializer being failable;
many domain initializers in the stdlib don't indicate that they can
fail directly in the name.

The point I'm trying to make is that the implications of “validating”
aren't clear, not that I can't tell whether it's failable.

In this change, for example:
https://github.com/apple/swift/compare/master...zwaldowski:string-from-code-units#diff-d39ec2c4819c950aeef95f899f53b104R79
I find the old code much clearer than the new code (I'm not sure why
_decode starts with an underscore here; isn't it part of your proposed
public API?)

The PR is ahead of the proposal here. It was pointed out in the review
thread up to this point that that that "primitive" isn't truly needed;
the initializers on String should be the API used by all.

Okay, well, proposals (including any diffs they reference) should remain
stable under review so we know what we're reviewing. I really don't
know what's being proposed here.

static func decode<
  Encoding: UnicodeCodecType,
  Input: CollectionType where Input.Generator.Element == Encoding.CodeUnit
>(
   _: Input, as: Encoding.Type, repairingInvalidCodeUnits: Bool = default
) -> (result: String, repairsMade: Bool)?

It seems like you are withdrawing decode from the proposal?

For convenience, the Bool flag here is also separated out to a more
common-case pair of String initializers:

I don't understand what it means for a “bool flag to be separated out to
a... pair of ... initializers.”

init<...>(codeUnits: Input, as: Encoding.Type)
init?<...>(validatingCodeUnits: Input, as: Encoding.Type)

Are you saying that the first initializer corresponds to
`repairingInvalidCodeUnits: true` and the second to
`repairingInvalidCodeUnits: false`?

What about the user who wants repairs if necessary and wants to know
about it when repairs were made?

Finally, for more direct compatibility with String.Type.fromCString(_:slight_smile:
and String.Type.fromCStringRepairingIllFormedUTF8(_:), these
constructors are overloaded for pointer-based strings of unknown length:

init(cString: UnsafePointer<CChar>)
init?(validatingCString: UnsafePointer<CChar>)

Why are we trying to “be compatible” with those static methods?

The ugliness in this instance is transitional;

What ugliness?

it stems from bridging the old-style constructor to the new-style
one.

I don't know what that means either.

The old signature's return type is awkward in that it returns the
extra hadError flag regardless of whether decoding failed, which is
discarded almost universally by clients in the stdlib.

I intend to revert the change to `fromCStringRepairingIllFormedUTF8`, or
elide `_decode` into it instead.

I don't know what that means either.

Lastly, I am very concerned about the “Alternatives Considered” section,
where, of one alternative, it says:

      This might be the better long-term solution from the perspective
      of API maintenance, but in the meantime this proposal has a fairly
      low impact.

We can't accept changes into the standard library “in the meantime,”
with the expectation that something more comprehensive will make them
obsolete. Even though we've had migration tools, we never operated that
way in the past, and as we head toward API and ABI stability it is even
more true today.

I understand the sentiment here, but I don't think "wait for the next
rewrite of String" is a good solution to a problem that the stdlib
already resolves.

I'm really confused. If the stdlib already resolves the problem, why is
there a proposal?

See below.

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

Yes.

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

W.r.t. direction, the fact that we have a major String overhaul planned
means that tackling this one corner of the API is probably not entirely
appropriate.

"Corner" is an unfair characterization;

Not at all; this addresses an important use-case, but accounts for a
very small part of the API.

users of Swift want to implement things to the level of safety and
performance bar set by the stdlib, but we are currently at a
disadvantage. No better example can be found but in
corelibs-foundation:
https://github.com/apple/swift-corelibs-foundation/blob/546dc8e16c3c34ca50f5752c6d0f39c3524f5f0a/Foundation/NSString.swift#L305.

Heh, well, that's ironic. Before Swift was released we had all of this
functionality for unicode transcoding available in the String API, and
it was decided that to avoid treading into Foundation's territory, any
functionality already exposed by NSString should not be public API on
String without Foundation loaded.

I have ambitions that Swift strings will be first-class standalone types
without the need to rely on Foundation some day soon, but under the
circumstances we may need a more comprehensive change than merely adding
fast unicode transcoding to justify treading into this area.

Is it not a deficiency of the stdlib when code has to resort to
non-public methods?

Yes. There are lots of deficiencies in the stdlib. I'm not saying this
shouldn't be addressed, but I'm concerned about addressing this in a
temporary way that we think may be sub-optimal.

The stdlib (i.e., the parts touched by the PR) and corelibs-foundation
would have to move in lockstep to adopt a replacement, so the existing
underscored versions are as good as public API.

I understand, and 100% encourage, the reticence around new API. However,
we've created a worse problem by encouraging slow, buggy, custom
versions of behavior that already exists in the stdlib, or implying that
the underscored API should be used because we haven't managed something
better yet. :confused:

I am not reticent to have new API. I am reticent to accept partial or
known-suboptimal temporary solutions when we are developing a
comprehensive plan that ought to address the same problems (among many
others).

···

on Mon Feb 15 2016, Zach Waldowski <swift-evolution@swift.org> wrote:

On Mon, Feb 15, 2016, at 11:48 AM, Dave Abrahams via swift-evolution > wrote:

on Thu Feb 11 2016, Douglas Gregor <swift-evolution@swift.org> wrote:

--
-Dave


(Zachary Waldowski) #18

Responses are inline.

Best,
Zachary Waldowski
zach@waldowski.me

> See responses inline.
>
> Aside: omgomgomg I got Dave Abrahams to respond to a proposal of mine

Don't worry; it'll wear off soon enough...

> Excitedly,
> Zachary Waldowski
> zach@waldowski.me
>
>>
>>
>> > Hello Swift community,
>> >
>> > The review of SE-0027 "Expose code unit initializers on String" begins
>> > now and runs through February 16, 2016. The proposal is available
>> > here:
>> >
>> > https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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
>> > <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/0027-string-from-code-units.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?
>>
>> I support the intent of the proposal but am -1 on the specific proposal
>> as offered.
>>
>> First, in the presentation, there are several things that make it hard
>> to evaluate:
>>
>> 1. The "decode" function signature is so wide that it can't be read
>> without scrolling at
>> https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.md
>>
>> 2. The proposal doesn't show any usage of the proposed APIs, so it
>> is hard to understand what effect these APIs would have on real
>> code. There are some examples of uses in
>> https://github.com/apple/swift/compare/master...zwaldowski:string-from-code-units
>> but I have to dig for them.
>>
>> 3. The description of the “Detailed Design” doesn't show any examples
>> either, so I have to imagine what “backporting the Swift 3.0 versions
>> of the CString constructors, then making them generic over their
>> input and codec” means.
>
> The notes are appreciated. I'll take care to add examples and fix
> formatting. The proposal was always intended to be paired with the PR to
> the stdlib, and that lack of clarity did not come up during the proposal
> vetting stage.
>
>> Next, when I look at *uses* that I can find (in the tests), I don't find
>> them to be clear.
>>
>> String(validatingCodeUnits: result, as: UTF8.self)
>>
>> What does “validatingCodeUnits” mean? Clearly we're going to do some
>> checking. Is there a repair? Is the initializer failable?
>
> I'm open to suggestions. The names as they are were lifted from the
> changes to the CString APIs made on swift-3-api-guidelines.

I suspected that they might be so I went and checked on GitHub... but
now I see that only searches the default branch :stuck_out_tongue:

    https://help.github.com/articles/searching-code/

But if the naming error was on our side, my apologies. We should do
better, regardless.

> "repairingCodeUnits" implies to me that there is something already wrong
> with them.

That doesn't bother me. If you are requesting the repair, you are
effectively assuming that there is something already wrong with them.

> I'm also not sure as to the point of the initializer being failable;
> many domain initializers in the stdlib don't indicate that they can
> fail directly in the name.

The point I'm trying to make is that the implications of “validating”
aren't clear, not that I can't tell whether it's failable.

Okay, point taken. Is your recommendation behind "repairingCodeUnits"?
Or something else?

>>
>> In this change, for example:
>> https://github.com/apple/swift/compare/master...zwaldowski:string-from-code-units#diff-d39ec2c4819c950aeef95f899f53b104R79
>> I find the old code much clearer than the new code (I'm not sure why
>> _decode starts with an underscore here; isn't it part of your proposed
>> public API?)
>
> The PR is ahead of the proposal here. It was pointed out in the review
> thread up to this point that that that "primitive" isn't truly needed;
> the initializers on String should be the API used by all.

Okay, well, proposals (including any diffs they reference) should remain
stable under review so we know what we're reviewing. I really don't
know what's being proposed here.

Noted. I was following the example set by many other reviews, and I
apologize for not knowing what the right was here.

I intend to continue to push changes to the PR (or make a new branch,
more likely), but will strive to make the proposal not dependent on the
branch anyway.

> static func decode<
> Encoding: UnicodeCodecType,
> Input: CollectionType where Input.Generator.Element == Encoding.CodeUnit
> >(
> _: Input, as: Encoding.Type, repairingInvalidCodeUnits: Bool = default
> ) -> (result: String, repairsMade: Bool)?

It seems like you are withdrawing decode from the proposal?

> For convenience, the Bool flag here is also separated out to a more
> common-case pair of String initializers:

I don't understand what it means for a “bool flag to be separated out to
a... pair of ... initializers.”

> init<...>(codeUnits: Input, as: Encoding.Type)
> init?<...>(validatingCodeUnits: Input, as: Encoding.Type)

Are you saying that the first initializer corresponds to
`repairingInvalidCodeUnits: true` and the second to
`repairingInvalidCodeUnits: false`?

Yes.

What about the user who wants repairs if necessary and wants to know
about it when repairs were made?

That's valid enough reason to keep the full version of decode around,
though I will again note that the flag is typically ignored. I've yet to
find someone actually using it.

> Finally, for more direct compatibility with String.Type.fromCString(_:slight_smile:
> and String.Type.fromCStringRepairingIllFormedUTF8(_:), these
> constructors are overloaded for pointer-based strings of unknown length:
>
> init(cString: UnsafePointer<CChar>)
> init?(validatingCString: UnsafePointer<CChar>)

Why are we trying to “be compatible” with those static methods?

They are existing public API and they are being deprecated and replaced
by this proposal. It was noted during the proposal shopping period that
requiring strlen and creating an UnsafeBufferPointer would be, in as
many words, too much migration effort.

> The ugliness in this instance is transitional;

What ugliness?

"I find the old code much clearer than the new code", being the
forwarding implementation of
String.Type.fromCStringRepairingIllFormedUTF8(_:).

> it stems from bridging the old-style constructor to the new-style
> one.

I don't know what that means either.

String.Type.fromCString(_:slight_smile: and
String.Type.fromCStringRepairingIllFormedUTF8(_:slight_smile: are replaced by
String(validatingCString:) and String(cString:), respectively, as
described under "Impact on existing code".

> The old signature's return type is awkward in that it returns the
> extra hadError flag regardless of whether decoding failed, which is
> discarded almost universally by clients in the stdlib.
>
> I intend to revert the change to `fromCStringRepairingIllFormedUTF8`, or
> elide `_decode` into it instead.

I don't know what that means either.

You found the changes fromCStringRepairingIllFormedUTF8 unclear, I
intend to return it to the previous version.

>> Lastly, I am very concerned about the “Alternatives Considered” section,
>> where, of one alternative, it says:
>>
>> This might be the better long-term solution from the perspective
>> of API maintenance, but in the meantime this proposal has a fairly
>> low impact.
>>
>> We can't accept changes into the standard library “in the meantime,”
>> with the expectation that something more comprehensive will make them
>> obsolete. Even though we've had migration tools, we never operated that
>> way in the past, and as we head toward API and ABI stability it is even
>> more true today.
>
> I understand the sentiment here, but I don't think "wait for the next
> rewrite of String" is a good solution to a problem that the stdlib
> already resolves.

I'm really confused. If the stdlib already resolves the problem, why is
there a proposal?

It has implementations in underscored form, which I can't use.

> See below.
>
>>
>> > Is the problem being addressed significant enough to warrant a change
>> > to Swift?
>>
>> Yes.
>>
>> > Does this proposal fit well with the feel and direction of Swift?
>>
>> W.r.t. direction, the fact that we have a major String overhaul planned
>> means that tackling this one corner of the API is probably not entirely
>> appropriate.
>
> "Corner" is an unfair characterization;

Not at all; this addresses an important use-case, but accounts for a
very small part of the API.

> users of Swift want to implement things to the level of safety and
> performance bar set by the stdlib, but we are currently at a
> disadvantage. No better example can be found but in
> corelibs-foundation:
> https://github.com/apple/swift-corelibs-foundation/blob/546dc8e16c3c34ca50f5752c6d0f39c3524f5f0a/Foundation/NSString.swift#L305.

Heh, well, that's ironic. Before Swift was released we had all of this
functionality for unicode transcoding available in the String API, and
it was decided that to avoid treading into Foundation's territory, any
functionality already exposed by NSString should not be public API on
String without Foundation loaded.

That's unfortunate.

I have ambitions that Swift strings will be first-class standalone types
without the need to rely on Foundation some day soon, but under the
circumstances we may need a more comprehensive change than merely adding
fast unicode transcoding to justify treading into this area.

> Is it not a deficiency of the stdlib when code has to resort to
> non-public methods?

Yes. There are lots of deficiencies in the stdlib. I'm not saying this
shouldn't be addressed, but I'm concerned about addressing this in a
temporary way that we think may be sub-optimal.

> The stdlib (i.e., the parts touched by the PR) and corelibs-foundation
> would have to move in lockstep to adopt a replacement, so the existing
> underscored versions are as good as public API.
>
> I understand, and 100% encourage, the reticence around new API. However,
> we've created a worse problem by encouraging slow, buggy, custom
> versions of behavior that already exists in the stdlib, or implying that
> the underscored API should be used because we haven't managed something
> better yet. :confused:

I am not reticent to have new API. I am reticent to accept partial or
known-suboptimal temporary solutions when we are developing a
comprehensive plan that ought to address the same problems (among many
others).

Not it's my turn to be confused. I don't think of this proposal as a
temporary solution, and would like more color on that if you would. Even
if a "more complete" variant found its way to the String views, the
number of clients in the stdlib and Foundation make it clear that this
group of constructors are necessary. Unless the entire UnicodeCodec
concept is going away, I don't see this proposal causing a drastic
migration problem; if they are, then the proposed constructors would be
just a small component of another heroic migration anyway.

···

On Mon, Feb 15, 2016, at 02:10 PM, Dave Abrahams via swift-evolution wrote:

on Mon Feb 15 2016, Zach Waldowski <swift-evolution@swift.org> wrote:
> On Mon, Feb 15, 2016, at 11:48 AM, Dave Abrahams via swift-evolution > > wrote:
>> on Thu Feb 11 2016, Douglas Gregor <swift-evolution@swift.org> wrote:

--
-Dave

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


(Charles Kissinger) #19

Dave, a few questions below:

Heh, well, that's ironic. Before Swift was released we had all of this
functionality for unicode transcoding available in the String API, and
it was decided that to avoid treading into Foundation's territory, any
functionality already exposed by NSString should not be public API on
String without Foundation loaded.

Could you explain what went into the decisions about what overlapping functionality was allowed and what was not? There seems to be significant overlapping functionality: fromCString, uppercaseString, and hasPrefix just to name a few redundant methods.

Also, the new functions in the proposal operate on parameters that are Swift types with no equivalent in Obj-C. Is that really treading into Foundation territory?

I have ambitions that Swift strings will be first-class standalone types
without the need to rely on Foundation some day soon, but under the
circumstances we may need a more comprehensive change than merely adding
fast unicode transcoding to justify treading into this area.

Are all of the Swift stdlib types that bridge to Foundation types (Array, Dictionary, etc.) under similar restrictions about treading on Foundation? I’m surprised by that approach.

Is it not a deficiency of the stdlib when code has to resort to
non-public methods?

Yes. There are lots of deficiencies in the stdlib. I'm not saying this
shouldn't be addressed, but I'm concerned about addressing this in a
temporary way that we think may be sub-optimal.

Is it the interface or the implementation that you think might be suboptimal? As long as we can settle on a good API, we don’t have to worry about the implementation changing underneath it, do we?

The stdlib (i.e., the parts touched by the PR) and corelibs-foundation
would have to move in lockstep to adopt a replacement, so the existing
underscored versions are as good as public API.

I understand, and 100% encourage, the reticence around new API. However,
we've created a worse problem by encouraging slow, buggy, custom
versions of behavior that already exists in the stdlib, or implying that
the underscored API should be used because we haven't managed something
better yet. :confused:

I am not reticent to have new API. I am reticent to accept partial or
known-suboptimal temporary solutions when we are developing a
comprehensive plan that ought to address the same problems (among many
others).

Is the comprehensive String redesign happening in public? I haven’t found anything, but I probably just don’t know where to look.

—CK

···

On Feb 15, 2016, at 11:10 AM, Dave Abrahams via swift-evolution <swift-evolution@swift.org> wrote:

--
-Dave

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


(Dave Abrahams) #20

Responses are inline.

Best,
Zachary Waldowski
zach@waldowski.me

> See responses inline.
>
> Aside: omgomgomg I got Dave Abrahams to respond to a proposal of mine

Don't worry; it'll wear off soon enough...

> Excitedly,
> Zachary Waldowski
> zach@waldowski.me
>
>>
>>
>> > Hello Swift community,
>> >
>> > The review of SE-0027 "Expose code unit initializers on String" begins
>> > now and runs through February 16, 2016. The proposal is available
>> > here:
>> >
>> > https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.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
>> > <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/0027-string-from-code-units.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?
>>
>> I support the intent of the proposal but am -1 on the specific proposal
>> as offered.
>>
>> First, in the presentation, there are several things that make it hard
>> to evaluate:
>>
>> 1. The "decode" function signature is so wide that it can't be read
>> without scrolling at
>> https://github.com/apple/swift-evolution/blob/master/proposals/0027-string-from-code-units.md
>>
>> 2. The proposal doesn't show any usage of the proposed APIs, so it
>> is hard to understand what effect these APIs would have on real
>> code. There are some examples of uses in
>> https://github.com/apple/swift/compare/master...zwaldowski:string-from-code-units
>> but I have to dig for them.
>>
>> 3. The description of the “Detailed Design” doesn't show any examples
>> either, so I have to imagine what “backporting the Swift 3.0 versions
>> of the CString constructors, then making them generic over their
>> input and codec” means.
>
> The notes are appreciated. I'll take care to add examples and fix
> formatting. The proposal was always intended to be paired with the PR to
> the stdlib, and that lack of clarity did not come up during the proposal
> vetting stage.
>
>> Next, when I look at *uses* that I can find (in the tests), I don't find
>> them to be clear.
>>
>> String(validatingCodeUnits: result, as: UTF8.self)
>>
>> What does “validatingCodeUnits” mean? Clearly we're going to do some
>> checking. Is there a repair? Is the initializer failable?
>
> I'm open to suggestions. The names as they are were lifted from the
> changes to the CString APIs made on swift-3-api-guidelines.

I suspected that they might be so I went and checked on GitHub... but
now I see that only searches the default branch :stuck_out_tongue:

    https://help.github.com/articles/searching-code/

But if the naming error was on our side, my apologies. We should do
better, regardless.

> "repairingCodeUnits" implies to me that there is something already wrong
> with them.

That doesn't bother me. If you are requesting the repair, you are
effectively assuming that there is something already wrong with them.

> I'm also not sure as to the point of the initializer being failable;
> many domain initializers in the stdlib don't indicate that they can
> fail directly in the name.

The point I'm trying to make is that the implications of “validating”
aren't clear, not that I can't tell whether it's failable.

Okay, point taken. Is your recommendation behind "repairingCodeUnits"?

If repairingInvalidCodeUnits reflects the correct semantics, that's what
I'd use.

Or something else?

>>
>> In this change, for example:
>> https://github.com/apple/swift/compare/master...zwaldowski:string-from-code-units#diff-d39ec2c4819c950aeef95f899f53b104R79
>> I find the old code much clearer than the new code (I'm not sure why
>> _decode starts with an underscore here; isn't it part of your proposed
>> public API?)
>
> The PR is ahead of the proposal here. It was pointed out in the review
> thread up to this point that that that "primitive" isn't truly needed;
> the initializers on String should be the API used by all.

Okay, well, proposals (including any diffs they reference) should remain
stable under review so we know what we're reviewing. I really don't
know what's being proposed here.

Noted. I was following the example set by many other reviews, and I
apologize for not knowing what the right was here.

n/p.

I intend to continue to push changes to the PR (or make a new branch,
more likely), but will strive to make the proposal not dependent on the
branch anyway.

> static func decode<
> Encoding: UnicodeCodecType,
> Input: CollectionType where Input.Generator.Element == Encoding.CodeUnit
> >(
> _: Input, as: Encoding.Type, repairingInvalidCodeUnits: Bool = default
> ) -> (result: String, repairsMade: Bool)?

It seems like you are withdrawing decode from the proposal?

?

> For convenience, the Bool flag here is also separated out to a more
> common-case pair of String initializers:

I don't understand what it means for a “bool flag to be separated out to
a... pair of ... initializers.”

> init<...>(codeUnits: Input, as: Encoding.Type)
> init?<...>(validatingCodeUnits: Input, as: Encoding.Type)

Are you saying that the first initializer corresponds to
`repairingInvalidCodeUnits: true` and the second to
`repairingInvalidCodeUnits: false`?

Yes.

Okay, well the language you used doesn't make that clear.

What about the user who wants repairs if necessary and wants to know
about it when repairs were made?

That's valid enough reason to keep the full version of decode around,
though I will again note that the flag is typically ignored. I've yet to
find someone actually using it.

Does the standard library use it?

> Finally, for more direct compatibility with String.Type.fromCString(_:slight_smile:
> and String.Type.fromCStringRepairingIllFormedUTF8(_:), these
> constructors are overloaded for pointer-based strings of unknown length:
>
> init(cString: UnsafePointer<CChar>)
> init?(validatingCString: UnsafePointer<CChar>)

Why are we trying to “be compatible” with those static methods?

They are existing public API and they are being deprecated and replaced
by this proposal. It was noted during the proposal shopping period that
requiring strlen and creating an UnsafeBufferPointer would be, in as
many words, too much migration effort.

Ah.

> The ugliness in this instance is transitional;

What ugliness?

"I find the old code much clearer than the new code", being the
forwarding implementation of
String.Type.fromCStringRepairingIllFormedUTF8(_:).

I'm sorry, I don't mean to be difficult, really, but I don't understand
how that answers my quetsion.

> it stems from bridging the old-style constructor to the new-style
> one.

I don't know what that means either.

String.Type.fromCString(_:slight_smile: and
String.Type.fromCStringRepairingIllFormedUTF8(_:slight_smile: are replaced by
String(validatingCString:) and String(cString:), respectively, as
described under "Impact on existing code".

Nor that.

> The old signature's return type is awkward in that it returns the
> extra hadError flag regardless of whether decoding failed, which is
> discarded almost universally by clients in the stdlib.
>
> I intend to revert the change to `fromCStringRepairingIllFormedUTF8`, or
> elide `_decode` into it instead.

I don't know what that means either.

You found the changes fromCStringRepairingIllFormedUTF8 unclear, I
intend to return it to the previous version.

>> Lastly, I am very concerned about the “Alternatives Considered” section,
>> where, of one alternative, it says:
>>
>> This might be the better long-term solution from the perspective
>> of API maintenance, but in the meantime this proposal has a fairly
>> low impact.
>>
>> We can't accept changes into the standard library “in the meantime,”
>> with the expectation that something more comprehensive will make them
>> obsolete. Even though we've had migration tools, we never operated that
>> way in the past, and as we head toward API and ABI stability it is even
>> more true today.
>
> I understand the sentiment here, but I don't think "wait for the next
> rewrite of String" is a good solution to a problem that the stdlib
> already resolves.

I'm really confused. If the stdlib already resolves the problem, why is
there a proposal?

It has implementations in underscored form, which I can't use.

> See below.
>
>>
>> > Is the problem being addressed significant enough to warrant a change
>> > to Swift?
>>
>> Yes.
>>
>> > Does this proposal fit well with the feel and direction of Swift?
>>
>> W.r.t. direction, the fact that we have a major String overhaul planned
>> means that tackling this one corner of the API is probably not entirely
>> appropriate.
>
> "Corner" is an unfair characterization;

Not at all; this addresses an important use-case, but accounts for a
very small part of the API.

> users of Swift want to implement things to the level of safety and
> performance bar set by the stdlib, but we are currently at a
> disadvantage. No better example can be found but in
> corelibs-foundation:
> https://github.com/apple/swift-corelibs-foundation/blob/546dc8e16c3c34ca50f5752c6d0f39c3524f5f0a/Foundation/NSString.swift#L305.

Heh, well, that's ironic. Before Swift was released we had all of this
functionality for unicode transcoding available in the String API, and
it was decided that to avoid treading into Foundation's territory, any
functionality already exposed by NSString should not be public API on
String without Foundation loaded.

That's unfortunate.

I have ambitions that Swift strings will be first-class standalone types
without the need to rely on Foundation some day soon, but under the
circumstances we may need a more comprehensive change than merely adding
fast unicode transcoding to justify treading into this area.

> Is it not a deficiency of the stdlib when code has to resort to
> non-public methods?

Yes. There are lots of deficiencies in the stdlib. I'm not saying this
shouldn't be addressed, but I'm concerned about addressing this in a
temporary way that we think may be sub-optimal.

> The stdlib (i.e., the parts touched by the PR) and corelibs-foundation
> would have to move in lockstep to adopt a replacement, so the existing
> underscored versions are as good as public API.
>
> I understand, and 100% encourage, the reticence around new API. However,
> we've created a worse problem by encouraging slow, buggy, custom
> versions of behavior that already exists in the stdlib, or implying that
> the underscored API should be used because we haven't managed something
> better yet. :confused:

I am not reticent to have new API. I am reticent to accept partial or
known-suboptimal temporary solutions when we are developing a
comprehensive plan that ought to address the same problems (among many
others).

Not it's my turn to be confused. I don't think of this proposal as a
temporary solution, and would like more color on that if you would.

You have said that in the fullness of time this problem would be better
addressed a different way.

Even if a "more complete" variant found its way to the String views,
the number of clients in the stdlib and Foundation make it clear that
this group of constructors are necessary. Unless the entire
UnicodeCodec concept is going away, I don't see this proposal causing
a drastic migration problem; if they are, then the proposed
constructors would be just a small component of another heroic
migration anyway.

We're not going to have that many more heroic migrations; that's my
point. We should approve the right answer, whatever that is.

···

on Mon Feb 15 2016, Zach Waldowski <swift-evolution@swift.org> wrote:

On Mon, Feb 15, 2016, at 02:10 PM, Dave Abrahams via swift-evolution > wrote:

on Mon Feb 15 2016, Zach Waldowski <swift-evolution@swift.org> wrote:
> On Mon, Feb 15, 2016, at 11:48 AM, Dave Abrahams via swift-evolution >> > wrote:
>> on Thu Feb 11 2016, Douglas Gregor <swift-evolution@swift.org> wrote:

--
-Dave