[Review #2] SE-0089: Renaming String.init<T>(_: T)


(Chris Lattner) #1

Hello Swift community,

The review of the revised proposal SE-0089: "Renaming String.init<T>(_: T)" begins now and runs through June 7. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0089-rename-string-reflection-init.md

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

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

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

What goes into a review?

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

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

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

Thank you,

-Chris Lattner
Review Manager


(Jacob Bandes-Storch) #2

(updating my evaluation of the first version of this proposal)

https://github.com/apple/swift-evolution/blob/master/proposals/0089-rename-string-reflection-init.md

        * What is your evaluation of the proposal?

+1, because:
  - I believe the default/generic "printing" initializer is not commonly
used (at least, not on purpose).
  - The new argument label clearly indicates what the initializer does.
  - The "lossless" protocol will be a welcome addition for
serialization/deserialization code, and it makes the replacement init(_:slight_smile:
API very clear in its behavior.

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

Yes, it fits well with the API guidelines.

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

Reviewed the initial proposal, followed and participated in the ensuing
discussion, and glanced at this second draft to remind myself of the final
changes.

- Jacob


(Ben Rimmington) #3

<https://github.com/apple/swift-evolution/blob/master/proposals/
0089-rename-string-reflection-init.md>

Instead of LosslessStringConvertible, could the existing Streamable be used?

    extension String {

        public init<T : Streamable>(_ streamable: T) {
            self.init()
            streamable.write(to: &self)
        }
    }

AFAIK, only three types (UnicodeScalar, Character, String) are streamable.
But other types could add conformance; otherwise why does Streamable exist?

Streamable could be renamed to OutputStreamable, so that InputStreamable
can be added later.

-- Ben


(Lily Ballard) #4

  * What is your evaluation of the proposal?

+1 in general, but I still think String.init(describing:) is annoyingly long and would prefer String.init(from:).

Also, I'm a little concerned by this bit:

As a performance optimization, the implementation of the string literal interpolation syntax will be changed to prefer the unlabeled initializer when interpolating a type that is LosslessStringConvertible or that otherwise has an unlabeled String initializer, but use the String.init<T>(describing: T) initializer if not.

Right now string interpolation is done via the StringInterpolationConvertible protocol, and in particular the implementation for String contains

  public init<T>(stringInterpolationSegment expr: T) {
    self = String(expr)
  }

This implementation could be changed to test for LosslessStringConvertible, but there's no way to test for "Does String have an unlabelled initializer that accepts type T?", so the only way to implement this proposed optimization is by adding extra compiler magic.

I'm also not convinced that this is a performance optimization at all. It seems reasonable to expect that the unlabeled initializer would basically be sugar for calling the labeled initializer (or accessing the description property directly), which means there's no performance benefit to calling the unlabeled initializer. I can't think of any good reason for a type to implement an unlabeled String initializer that's more efficient than its implementation of CustomStringConvertible or Streamable.

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

Yes.

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

A quick reading of the current proposal, and I participated in the original review.

-Kevin Ballard

···

On Sat, Jun 4, 2016, at 11:24 AM, Chris Lattner via swift-evolution wrote:


(Vladimir) #5

The question regarding FloatingPoint.

Now we have:
var float = 1.0/3.0
var s = "\(float)"
print(s) // 0.333333333333333

What we'll have after the proposal implemented?

As stated in proposal: Standard library types to conform -> The following standard library types and protocols should be changed to conform to LosslessStringConvertible -> "FP types should be able to conform. There are algorithms that are guaranteed to turn IEEE floating point values into a decimal representation in a reversible way. I don’t think we care about NaN payloads, but an encoding could be created for them as well."

So, are we going to change what representation the float value will have as result of string interpolation? If so, how to 'get' this old plain "0.333333333333333" ?

···

On 04.06.2016 21:24, Chris Lattner via swift-evolution wrote:

Hello Swift community,

The review of the revised proposal SE-0089: "Renaming String.init<T>(_: T)" begins now and runs through June 7. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0089-rename-string-reflection-init.md

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

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

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

What goes into a review?

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

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

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

Thank you,

-Chris Lattner
Review Manager

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


(David Sweeris) #6

Speaking of which, would "Serializable" be a better name for the protocol? I don't recall whether anyone's already asked.

- Dave Sweeris

···

On Jun 4, 2016, at 15:46, Jacob Bandes-Storch via swift-evolution <swift-evolution@swift.org> wrote:

  - The "lossless" protocol will be a welcome addition for serialization/deserialization code, and it makes the replacement init(_:slight_smile: API very clear in its behavior.


(Patrick Smith) #7

The issue I think is that it would open up serialisation for all sorts of formats, which is a much larger problem in itself, whereas this can just focus on a user-defined ‘lossless’ string.

Patrick

···

On 5 Jun 2016, at 1:25 PM, David Sweeris via swift-evolution <swift-evolution@swift.org> wrote:

On Jun 4, 2016, at 15:46, Jacob Bandes-Storch via swift-evolution <swift-evolution@swift.org> wrote:

- The "lossless" protocol will be a welcome addition for serialization/deserialization code, and it makes the replacement init(_:slight_smile: API very clear in its behavior.

Speaking of which, would "Serializable" be a better name for the protocol? I don't recall whether anyone's already asked.

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


(Vladimir) #8

In this case not just "Serializable" but "StringSerializable", no?

···

On 05.06.2016 6:25, David Sweeris via swift-evolution wrote:

On Jun 4, 2016, at 15:46, Jacob Bandes-Storch via swift-evolution <swift-evolution@swift.org> wrote:

  - The "lossless" protocol will be a welcome addition for serialization/deserialization code, and it makes the replacement init(_:slight_smile: API very clear in its behavior.

Speaking of which, would "Serializable" be a better name for the protocol? I don't recall whether anyone's already asked.

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


(David Sweeris) #9

Good point

···

Sent from my iPhone

On Jun 5, 2016, at 00:16, Patrick Smith <pgwsmith@gmail.com> wrote:

The issue I think is that it would open up serialisation for all sorts of formats, which is a much larger problem in itself, whereas this can just focus on a user-defined ‘lossless’ string.

Patrick

On 5 Jun 2016, at 1:25 PM, David Sweeris via swift-evolution <swift-evolution@swift.org> wrote:

On Jun 4, 2016, at 15:46, Jacob Bandes-Storch via swift-evolution <swift-evolution@swift.org> wrote:

- The "lossless" protocol will be a welcome addition for serialization/deserialization code, and it makes the replacement init(_:slight_smile: API very clear in its behavior.

Speaking of which, would "Serializable" be a better name for the protocol? I don't recall whether anyone's already asked.

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