[Review] SE-0166: Swift Archival & Serialization


(Douglas Gregor) #1

Hello Swift community,

The review of SE-0166 "Swift Archival & Serialization" begins now and runs through April 12, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.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/0166-swift-archival-serialization.md
Reply text
Other replies
<https://github.com/apple/swift-evolution/blob/master/process.md#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

Review Manager


(Johannes Lund) #2

Hi!

To begin with, I have three questions:
1. Are these proposals purely meant for archival or do they also aim to be the basis of an optimal solution for decoding/unmarshalling JSON from external APIs?
2. How are Optionals of Codable types handled? I see no mentions of this. Do they map to missing keys and/or null, or are they not handled automatically at all?
3. If you’re writing a lot of custom decoding code, how are you supposed to debug errors? Do the error messages contain metadata? (I see no traces of it, but could happen behind the scenes) Or should you rely merely on Swift Error breakpoints?

/Johannes Lund

···

6 apr. 2017 kl. 20:10 skrev Douglas Gregor <dgregor@apple.com>:

Hello Swift community,

The review of SE-0166 "Swift Archival & Serialization" begins now and runs through April 12, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.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/0166-swift-archival-serialization.md
Reply text
Other replies
<https://github.com/apple/swift-evolution/blob/master/process.md#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

Review Manager

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


(Marco Masser) #3

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.md

I’m not giving a complete review here, but this proposal seems like a fantastic solution to me.
There’s one thing I either missed or that is actually missing that I would like to discuss.

NSObject defines the following method:
func awakeAfter(using aDecoder: NSCoder) -> Any?

I can find no discussion in the proposal about the intended replacement for the functionality this method provides. This method probably counts to the more obscure parts of Foundation and is probably almost never implemented. But there’s a use case in the code base I’m working on and I would like to discuss the intended replacement mechanism for it in the context of this proposal.

We have an Objective-C class that represents a single DNS name (e.g. a host name or a domain name). Instances are cached so whenever such an object is created (for e.g. “apple.com”), the same instance is returned (this is done using factory methods).
These DNS name objects are also encoded using NSCoding (for distributed object calls and for archiving to disk) and when decoding them, the same instance is returned every time. The decoding mechanism cannot take care of this because it cannot know whether such an instance exists already (created by a previous call to the aforementioned factory method). Using awakeAfter(using:) provides this functionality by looking up the cache and returning the already existing instance.

I suspect that the replacement for such a use case in Swift are structs and value types where there is no concept of identity. I would like to know if that is the answer the proposal author would give or if I’m missing something.

Similarly, I can’t find a replacement for the functionality following method of NSObject:
- (nullable id)replacementObjectForCoder:(NSCoder *)aCoder;

… but I think this is only used for DO calls and proxy objects and the intended replacement is XPC.

Regards,

Marco

···

On 2017-04-06, at 20:10, Douglas Gregor <dgregor@apple.com> wrote:

Hello Swift community,

The review of SE-0166 "Swift Archival & Serialization" begins now and runs through April 12, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.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/0166-swift-archival-serialization.md
Reply text
Other replies
<https://github.com/apple/swift-evolution/blob/master/process.md#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

Review Manager

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


(Colin Barrett) #4

Proposal Link:
https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.md

I'm a +1 on the proposal, I just have a few small questions and suggestions
after giving it a brief read.

1) I'm not thrilled about the name "unkeyedContainer()" and related names.
Would "sequentialContainer" or something along those lines be a better
name? The fact that it can encode several, heterogeneous values in a fixed
sequence seems to be the most salient feature of it that distinguishes it
from the keyed containers and the single value containers.

2) I feel like the reasoning behind the design for the CodingUserInfoKey
isn't clearly articulated, but it seems a bit odd to me. First off, I'm
confused why with the current design, the declaration isn't simply:

protocol CodingUserInfoKey: CodingKey, Hashable {}

I tried mocking something up with the shipping Swift 3.1, and got the
following error:

using 'CodingUserInfoKey' as a concrete type conforming to protocol

'Hashable' is not supported

So maybe it's just a question of lack of compiler support. Still, I feel
that the lack of a connection between the CodingKey and the
CodingUserInfoKey types is fairly confusing.

To be more concrete but way more speculative, I'm assuming that current
design one is intended to declare all their keys in an enum like,

enum SomeType.Key: CodingKey {
  case foo
  case bar
  // ...
  case metaKey1
  case metaKey2
}

And then write an init? extension for CodingUserInfoKey.

It seems more natural however to me to leverage the nested container
support instead. What exactly that design would look like is unclear to
me—do you return a value that implements Encoder? Do you add duplicate
nestedKeyedEncoder for the userInfo case? but it seems more of a clean fit
with the rest of the API than exposing a dictionary.

Both pretty minor issues when considered with all the great ideas in the
API. Looking forward to reviewing 0167 as well!

Cheers,
-C

···

On Thu, Apr 6, 2017 at 2:10 PM Douglas Gregor <dgregor@apple.com> wrote:

Hello Swift community,

The review of SE-0166 "Swift Archival & Serialization" begins now and runs
through April 12, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.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/0166-swift-archival-serialization.md

Reply text

Other replies

<https://github.com/apple/swift-evolution/blob/master/process.md#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

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


(Brent Royal-Gordon) #5

Proposal link:

https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.md

What is your evaluation of the proposal?

I've interrogated this proposal pretty thoroughly in the draft threads, and overall, I'd say it's excellent. I don't agree with every decision, but I think they are all very defensible.

If there is one thing I'm still not satisfied with, it's the large number of overloaded `encode` and `decode` methods; I think this design puts far too much effort into optimizing performance for rarely used types like `UInt16` when it would be better to write smaller, more compact interfaces that are easier to wrap your head around.

I'm also not too happy with the use of `mutating` on the containers. Some containers require mutability, others don't, it's impossible to remember which ones are which, and Swift will complain about you using `var` instead of `let` or vice versa if you get it wrong.

But these are minor nitpicks. This proposal will be a huge boon to Swift development and I support it wholeheartedly.

Incidentally, I think that in the future, we should try to infuse elements of alternative design 5 (where containers are scoped by closures). This points the way to a feature we ought to add to Swift: An `@once` annotation which promises that a closure parameter is executed exactly once, and (if the closure is not `@escaping`) can therefore be trusted by the definite initialization checker. We can then introduce closure-based versions of the container-fetching methods and have a nice, backwards-compatible improvement in ergonomics.

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

Absolutely. The lack of a good serialization solution is a major issue.

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

Mostly. There are things that don't, like the large and arbitrary set of primitive types and the treatment of Optionals, but I think these design decisions are defensible.

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

I think this is a much better design than NSCoding in many ways.

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

I think it's fair to say: A lot.

···

On Apr 6, 2017, at 11:10 AM, Douglas Gregor <dgregor@apple.com> wrote:

--
Brent Royal-Gordon
Architechies


(Russ Bishop) #6

Hello Swift community,

The review of SE-0166 "Swift Archival & Serialization" begins now and runs through April 12, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.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/0166-swift-archival-serialization.md
Reply text
Other replies
<https://github.com/apple/swift-evolution/blob/master/process.md#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?

It is a good first step but I feel like it tries to match NSSecureCoding design patterns too strongly and introduces a lot of potential for API misuse (convention rather than using the type system to our advantage). If we ask "what does a modern take on NSSecureCoding look like?", this proposal is the answer. If we ask "how can we do better than NSSecureCoding?" then this proposal needs a little bit of refinement.

Some examples:

* String and Int keys being optional, giving a CodingKey the opportunity to return nil for both at runtime.
* Encoder has three functions, but only one may ever be called. This API is the opposite of "pit of success".

There is also the problem of an implementation cliff: A type that has one non-Encodable property suddenly needs to provide a complete implementation. It is relatively common to have ephemeral data you don't even want to be encoded but again it seems like you need to jump immediately to a completely manual solution. There may not be a way to square this circle but it is worth thinking about. One improvement would be that a type can provide its own CodingKey but still get automatic conformance for all properties that match. At least then you would have a way to filter out the properties you don't want.

I don't understand why KeyedEncodingContainer needs all those overloads; automatic conformance to Encodable should be provided for the stdlib types in those overloads so why would they be necessary?

KeyedEncodingContainer.encodeWeak seems like it should be a protocol refinement so you can check for the capability (or potentially know at compile time). The decoder begs a similar question: why not rely on the generic functions? (One minor bit of bike shedding: decode/decodeIfPresent could instead be decode(required:) and decode(optional:)).

I really strongly dislike mixing up the Unkeyed and Keyed concepts. A type should need to explicitly opt-in to supporting unkeyed and that should be enforced at compile time. Unkeyed encoding is a potential versioning nightmare and should be handled with care.

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

Definitely yes.

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

See comments above

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

Yes; C#'s serialization attributes are a better and more comprehensive solution but we don't have custom attributes in Swift and property behaviors were deferred. This problem is too important to leave to the future though. If we did ever add custom attributes or if property behaviors get implemented then this design could adopt them incrementally without breaking compatibility (e.g. a serialization transformer behavior that turns a non-Encodable property into an Encodable one, or a behavior that ignores a property for serialization purposes).

I want to say thank-you to Itai, Michael, and Tony for their hard work on this and the related proposal; having done a proposal myself I know how much work it entails.

Russ Bishop

···

On Apr 6, 2017, at 11:10 AM, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:


(Brad Hilton) #7

-1. I support the motivation, but I think this proposal and SE-0167 are too premature.

Don’t get me wrong, I think that Swift leaves much to be desired as far as serialization/deserialization goes. However I think that these proposals are putting the cart before the horse. I think we first need to add genuine reflection APIs before adding encoding/decoding capabilities to the stdlib. Reflection will substantially simply most serialization/deserialization task. These APIs borrow too much Cocoa’s native archiving capabilities for my taste and I’d rather not introduce something that may be considered legacy in the future.

···

Hello Swift community,

The review of SE-0166 "Swift Archival&Serialization" begins now and runs through April 12, 2017. The proposal is available here:

> https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.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/0166-swift-archival-serialization.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

Review Manager


(Jon Shier) #8

What is your evaluation of the proposal?

-1. Like I said in my review of 167, this sort of core API is far too important for the implementation to be so flawed.

* This proposal is extensive. An implementation playground would’ve been nice. Containers seem fairly odd, so it would’ve been nice to see how they feel in use.

* Like 167, this proposal feels very Objective-C like. While we want interoperability with NSCoder and NSKeyedArchiver, this should be a native Swift solution first and a bridge second. Also, some bits of the API aren’t Swifty; any API which uses Type.self just doesn’t feel right. Please, use the generics.

someString = try container.decode(forKey: .someString) can infer the type from the assignee, no?

* This sort of API works well when the responses are logically formed, but this also forms the basis of the APIs we’ll be using to interact with JSON, which can be anything but logically formed. For instance:

struct Veritas {
    let truth: Bool
}

Simple, right?

But say it’s JSON representation is this:

{ “some_Bad_key” : “Yes” }

How would this proposal let me transform a Yes string into the proper boolean value? There doesn’t seem to be a solution provided, just the brute force method of trying to decode as a string first and then transforming. Plus, if I have a single value that I need to do something custom to, I need implement init(from decoder:) for my entire type, so it becomes very painful very quickly. I foresee a new collection of open source libraries to remedy this shortcoming, and they really shouldn’t have to.

Also, what if I wanted to be able to decode this type from both the terrible JSON and a nice on disk format? Some way to provide multiple conforming initializers would be nice.

Suffice to say my biggest concern here is that this API is designed for the ideal and not the messy reality of JSON APIs created by the lowest bidder. Since this proposal is supposed to cover both, it needs to offer support for the worst case scenarios more easily.

* The errors generated just aren’t good. I covered this in more detail in my review of 167, but suffice it to say they’re too limited by the desire to bridge them to NSErrors and don’t capture any useful details about what caused them in the first place. Also, they shouldn’t live in Foundation, as this is a feature the standard library will implement.

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

Yes, this is a problem worth solving, I just don’t think this proposal is good enough.

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

No. Passing types around isn’t Swifty at all. This fits right in in Objective-C though.

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

I’ve been an Argo user for 2 years, so I’m used to a certain terseness and flexibility in my decoders. This proposal brings neither. Argo also has superior errors which capture the keys and types that caused an issue. I guarantee there will be at least one open source library which provides operators that wrap this proposal’s API so that you can properly transform decoded values without multiple lines of code.

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

Multiple read throughs, about an hour of analysis.

Jon Shier


(Karl) #9

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?

-1

The proposal is mis-named; it isn’t Swift Archival and Serialisation. What it really is, is Foundation Archival and Serialisation. The proposal doesn’t even mention where these new protocols and types will live, but the general talk is all about Foundation and nothing at all about the standard library, so I’m assuming that’s where they will go.

- I fundamentally disagree with having the conformance be synthesised. We don’t do this anywhere else in the language, AFAIK. I believe it will make it easier for naive developers to suffer privacy leaks, or cause hard-to-debug crashes because they were persisting and restoring non-persistable values.

- I believe the proposal is premature. It is likely that Smart KeyPaths would allow us to define a collection of serialisable details of an instance in a convenient way which does not require compiler magic. Such a design could also allow more features than the one proposed (KeyPaths can select particular details from deep within properties, and the “collection" could be a Dictionary of KeyPaths to Serialisation Keys). A lot of effort has been made to make this all “magic” and automatic, but I see that as an anti-goal. I believe we are close to being able to write a safe, convenient serialisation API without the need for hidden and un-debuggable synthesis. This proposal should wait.

- Not only that, but I feel it would muddy the line between the Swift language and Foundation beyond what is acceptable. Those libraries are crucial to Apple’s platforms, so it’s understandable that you’re so protective of them, but I personally feel they are very far from what the “core libraries” of Swift could be. For example, having separate Data and DispatchData types, and no owned reference-counted buffer in the standard library at all is a horrible design IMO. Even minor pull-requests in to the core libraries seems to require herculean amounts of effort (https://github.com/apple/swift-corelibs-libdispatch/pull/172 https://github.com/apple/swift/pull/7446), so I have precisely 0 hope of anybody outside of Apple bringing any fresh ideas or improvements to the table.

I don’t want to go too much in to my gripes with Foundation and Dispatch, but suffice to say that giving them special language consideration is a step too far. This is Apple proposing a design that’s good for Apple and nobody else, and I’m not in favour of breaking layering even further to accomplish it.

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

No. I feel the solution is disproportionate to the problem.

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

It’s a poor design, only differing from existing libraries in how Apple-specific it is, and the principle-breaking language magic needed in order to achieve it. It does not fit at all with much better proposals, like Smart KeyPaths. Maybe the authors should have co-ordinated more with others before proposing it. And presenting an implementation PR at the same time just feels wrong IMO, like Apple expects to have this proposal rubber-stamped for their SDK plans.

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

I also develop for Android, and it’s extremely easy to implement serialisation using annotation-based libraries such as Icepick (https://github.com/frankiesardo/icepick).
I even feel like that is too much magic sometimes, but I’d prefer an annotation-based solution to invisible conformance synthesis any day of the week.

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

I read the previous discussions, and have read and re-read the proposal a couple of times.

···

More information about the Swift evolution process is available at

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

-Doug

Review Manager

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


(Tony Parker) #10

Hi everyone,

Thanks for your feedback on this proposal. Based on that plus additional feedback from core team members and others who responded off-thread, we are making the following small adjustments:

* KeyedEncoderContainerProtocol, KeyedDecodingContainerProtocol, UnkeyedEncodingContainer, UnkeyedDecodingContainer, SingleValueEncodingContainer, SingleValueDecodingContainer will drop their Data-taking functions. Data will conform to Codable, so it just goes through the normal paths like other types.
* The above will allow those protocols, plus Encodable, Decodable, typealias Codable, Encoder, Decoder, CodingKey, struct CodingUserInfoKey to be part of the standard library (not in Foundation), resolving the concern about reaching too far up the stack for the compiler.
* JSONEncoder/Decoder, PropertyListEncoder/Decoder remain in Foundation. These are concrete implementations of the above protocols, like the ones that will appear in many libraries, we hope.

We are not currently proposing using the integer protocols. The reasoning is that using them them confers a requirement upon all possible encoders and decoders to support arbitrary-width integers and floating point values. We’re not convinced this is feasible, but we will continue to evaluate.

If the core team accepts the modified proposal, we will get this merged as soon as possible with the goal of allowing all of you to try everything out via Swift’s frequent toolchain snapshots. We will be looking for feedback on how it works in more real world scenarios, and we will consider further adjustments to the API before Swift 4 is final. We are hoping that this extended trial period will allow us to make sure that everything works out the way we expected.

Thanks,
- Tony

···

On Apr 6, 2017, at 11:10 AM, Douglas Gregor <dgregor@apple.com> wrote:

Hello Swift community,

The review of SE-0166 "Swift Archival & Serialization" begins now and runs through April 12, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.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/0166-swift-archival-serialization.md
Reply text
Other replies
<https://github.com/apple/swift-evolution/blob/master/process.md#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

Review Manager

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


(Zachary Waldowski) #11

I want to disagree with this is strongly as possible lest it influence
the proposal in any way whatsoever. Just because you can solve something
through reflection doesn't mean you should.

Zachary Waldowski
zach@waldowski.me

···

On Wed, Apr 12, 2017, at 03:22 PM, Brad Hilton via swift-evolution wrote:

-1. I support the motivation, but I think this proposal and SE-0167 are
too premature.

Don’t get me wrong, I think that Swift leaves much to be desired as far
as serialization/deserialization goes. However I think that these
proposals are putting the cart before the horse. I think we first need to
add genuine reflection APIs before adding encoding/decoding capabilities
to the stdlib. Reflection will substantially simply most
serialization/deserialization task. These APIs borrow too much Cocoa’s
native archiving capabilities for my taste and I’d rather not introduce
something that may be considered legacy in the future.

> Hello Swift community,
>
>
> The review of SE-0166 "Swift Archival&Serialization" begins now and runs through April 12, 2017. The proposal is available here:
>
> > https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.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/0166-swift-archival-serialization.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
>
>
> Review Manager
>
>
>
>
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Brent Royal-Gordon) #12

* String and Int keys being optional, giving a CodingKey the opportunity to return nil for both at runtime.

This was true in the first public draft, but I believe in this version `stringValue` is no longer Optional.

* Encoder has three functions, but only one may ever be called. This API is the opposite of "pit of success".

Personally, I'm worried about that too. But I had a lot of trouble coming up with an alternative that didn't violate a more important goal, like "being able to throw errors" or "being compatible with classes".

Last-ditch suggestion: Change a bunch of names so that, for instance, `Encoder` is instead `EncodingContainerizer`. (That's a terrible name, but "Factory" gives me the hives.) That way, the name of the type gives you a hint that you're supposed to use it to make a container. You might even rename the methods to e.g. `useContainer(keyedBy:)`, which sound a little more stateful and mutually-exclusive.

  func encode(to encoder: EncodingContainerizer) throws {
    var container = encoder.useContainer(keyedBy: CodingKeys.self)
    try container.encode(name, forKey: .name)
    try container.encode(birthDate, forKey: .birthDate)
  }

I don't understand why KeyedEncodingContainer needs all those overloads; automatic conformance to Encodable should be provided for the stdlib types in those overloads so why would they be necessary?

I argued about this pretty vigorously. They want to avoid the overhead of building an encoder and single-value container and then making several generic calls to encode the value into the container. Personally, I think this is premature optimization of the highest order—particularly since building an encoder and single-value container are often no-ops—but this is the design they chose, and I don't think it's worth rejecting for that alone.

KeyedEncodingContainer.encodeWeak seems like it should be a protocol refinement so you can check for the capability (or potentially know at compile time).

This probably ends up duplicating not only `KeyedEncodingContainerProtocol`, but also `UnkeyedEncodingContainer`, `Encoder`, and `Encodable`. Doable, yes. Worth it, especially when `encodeWeak` has a pretty sensible fallback behavior? Eh.

(One minor bit of bike shedding: decode/decodeIfPresent could instead be decode(required:) and decode(optional:)).

I don't think `required:` and `optional:` are really helpful. The name here is kind of like `addingWithOverflow(_:_:)`—you really want it to be `adding(x, y, .withOverflow)`, but it's not worth creating a dummy enum just to make a method read properly. Similarly, you'd like `decode(Int.self, .ifPresent)`, but the dummy's not worth the better readability.

I'm not sure why we don't do this, though:

  // Just showing Unkeyed for simplicity
  
  func decode<T: Decodable>(_ type: T.Type) throws -> T
  func decode<T: Decodable>(_ type: Optional<T>.Type) throws -> T?

  let alwaysInt = try container.decode(Int.self)
  let maybeInt = try container.decode(Int?.self)

I really strongly dislike mixing up the Unkeyed and Keyed concepts. A type should need to explicitly opt-in to supporting unkeyed and that should be enforced at compile time. Unkeyed encoding is a potential versioning nightmare and should be handled with care.

I think they've done a reasonable job of putting unkeyed coding in a sharps drawer by making you specifically ask for it and giving it an ugly name.

C#'s serialization attributes are a better and more comprehensive solution but we don't have custom attributes in Swift and property behaviors were deferred. This problem is too important to leave to the future though. If we did ever add custom attributes or if property behaviors get implemented then this design could adopt them incrementally without breaking compatibility (e.g. a serialization transformer behavior that turns a non-Encodable property into an Encodable one, or a behavior that ignores a property for serialization purposes).

On the contrary, I think we *can* safely leave this to the future. If a future version of Swift and Foundation added:

  @uncoded var foo: Bar

That would be a completely additive change. Until then, there's an irritating but serviceable solution available—write your own CodingKeys enum and let code generation write the `Codable` conformances based on it.

···

On Apr 12, 2017, at 11:44 AM, Russ Bishop via swift-evolution <swift-evolution@swift.org> wrote:

--
Brent Royal-Gordon
Architechies


(David Hart) #13

Hello Swift community,

The review of SE-0166 "Swift Archival & Serialization" begins now and runs through April 12, 2017. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0166-swift-archival-serialization.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/0166-swift-archival-serialization.md
Reply text
Other replies
<https://github.com/apple/swift-evolution/blob/master/process.md#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?

It is a good first step but I feel like it tries to match NSSecureCoding design patterns too strongly and introduces a lot of potential for API misuse (convention rather than using the type system to our advantage). If we ask "what does a modern take on NSSecureCoding look like?", this proposal is the answer. If we ask "how can we do better than NSSecureCoding?" then this proposal needs a little bit of refinement.

I also agree that the API feels slightly more influenced by NSCoding and Objective-C paradigms than by Swift idioms.

Some examples:

* String and Int keys being optional, giving a CodingKey the opportunity to return nil for both at runtime.
* Encoder has three functions, but only one may ever be called. This API is the opposite of "pit of success”.

The amount of preconditions in the documentation of Coder and then CocoaError.coderTypeMismatch errors of Decoder paint the same picture. I understand the argument that writing this proposal taking full advantage of the type system would exponentially increase the number of types and worsen the API. But perhaps the Encoder/Decoder protocols could take advantage of more types.

There is also the problem of an implementation cliff: A type that has one non-Encodable property suddenly needs to provide a complete implementation. It is relatively common to have ephemeral data you don't even want to be encoded but again it seems like you need to jump immediately to a completely manual solution.

I don’t think you’re right here. The proposal states:

  • Types falling into (1) — and types which manually provide a CodingKey enum (named CodingKeys, directly, or via a typealias) whose cases map 1-to-1 to Encodable/Decodable properties by name — get automatic synthesis of init(from:) and encode(to:) as appropriate, using those properties and keys

So if you do have ephemeral data, you’d only need to define the CodingKey enum and be good to go.

There may not be a way to square this circle but it is worth thinking about. One improvement would be that a type can provide its own CodingKey but still get automatic conformance for all properties that match. At least then you would have a way to filter out the properties you don't want.

I don't understand why KeyedEncodingContainer needs all those overloads; automatic conformance to Encodable should be provided for the stdlib types in those overloads so why would they be necessary?

I’ve been beating the same bush. I know the arguments but it just feels wrong to me. That’s a case where this API definitely feels more like Objective-C than Swift.

···

On 12 Apr 2017, at 20:44, Russ Bishop via swift-evolution <swift-evolution@swift.org> wrote:

On Apr 6, 2017, at 11:10 AM, Douglas Gregor via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

KeyedEncodingContainer.encodeWeak seems like it should be a protocol refinement so you can check for the capability (or potentially know at compile time). The decoder begs a similar question: why not rely on the generic functions? (One minor bit of bike shedding: decode/decodeIfPresent could instead be decode(required:) and decode(optional:)).

I really strongly dislike mixing up the Unkeyed and Keyed concepts. A type should need to explicitly opt-in to supporting unkeyed and that should be enforced at compile time. Unkeyed encoding is a potential versioning nightmare and should be handled with care.

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

Definitely yes.

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

See comments above

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

Yes; C#'s serialization attributes are a better and more comprehensive solution but we don't have custom attributes in Swift and property behaviors were deferred. This problem is too important to leave to the future though. If we did ever add custom attributes or if property behaviors get implemented then this design could adopt them incrementally without breaking compatibility (e.g. a serialization transformer behavior that turns a non-Encodable property into an Encodable one, or a behavior that ignores a property for serialization purposes).

I want to say thank-you to Itai, Michael, and Tony for their hard work on this and the related proposal; having done a proposal myself I know how much work it entails.

Russ Bishop

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


(Brent Royal-Gordon) #14

* The above will allow those protocols, plus Encodable, Decodable, typealias Codable, Encoder, Decoder, CodingKey, struct CodingUserInfoKey to be part of the standard library (not in Foundation), resolving the concern about reaching too far up the stack for the compiler.

I think this is a huge win for Swift and a great move, but...

* KeyedEncoderContainerProtocol, KeyedDecodingContainerProtocol, UnkeyedEncodingContainer, UnkeyedDecodingContainer, SingleValueEncodingContainer, SingleValueDecodingContainer will drop their Data-taking functions. Data will conform to Codable, so it just goes through the normal paths like other types.

...I hope the `Data`-taking primitive will not be deleted, but rather replaced by one that takes `UnsafeRawBufferPointer` or some similar standard library representation of a byte sequence. Even if it has to be called `encodeBytes` and `decodeBytes` to clarify that it's not somehow coding a pointer, I think a series of bytes truly *is* a primitive type, and something fairly distinct from an unkeyed container of `UInt8`s.

···

On Apr 20, 2017, at 10:08 AM, Tony Parker via swift-evolution <swift-evolution@swift.org> wrote:

--
Brent Royal-Gordon
Architechies


(Karl) #15

That’s a great step; thanks a lot!

As for Data/UnsafeRawBufferPointer (beyond this proposal): I agree that a Data object which owns its buffer should be the common-currency for passing buffers around, but I also think that the standard library should own that “Data” type. Data is (IMO), no less "primitive” than, say, Array, Dictionary, Set and String - yet while all of those types are part of the standard library, Data lives much further down the stack, in Foundation. The standard library is limited to unsafe buffers, without a “safe” alternative. Even though Foundation is a core library and very useful, it is still an optional component of any Swift installation or application, and I feel it’s not unreasonable to ask for a safe buffer without that dependency.

I could of course define my own RAII, UnsafeRawBufferPointer-wrapping “Data” type, but then it either spreads throughout my API and undermines Data as a common currency, or I need to convert it (with a copy, in order to preserve value semantics). This is the same issue that affects DispatchData/Data bridging, which in that case undermines APIs such as DispatchIO; either all of my APIs now take a DispatchData (leaking implementation details), or I have to copy the buffer in to a Foundation Data.

That’s an issue which I would like to help solve more comprehensively one day, but for the mean time:

1) Is untyped data really a “primitive” in the context of serialisation? If so...
2) Why are we removing it? Is it only because the standard library lacks a good type to represent the data? If so...
3) Why not introduce a generic function?

mutating func encodeBytes<C>(_ bytes: C, forKey key: Key) throws where C: Collection, C.Element == UInt8

This single function would be able to encode Data, DispatchData, UnsafeRawBufferPointer, and even an Array<UInt8>.

- Karl

···

On 20 Apr 2017, at 19:08, Tony Parker via swift-evolution <swift-evolution@swift.org> wrote:

Hi everyone,

Thanks for your feedback on this proposal. Based on that plus additional feedback from core team members and others who responded off-thread, we are making the following small adjustments:

* KeyedEncoderContainerProtocol, KeyedDecodingContainerProtocol, UnkeyedEncodingContainer, UnkeyedDecodingContainer, SingleValueEncodingContainer, SingleValueDecodingContainer will drop their Data-taking functions. Data will conform to Codable, so it just goes through the normal paths like other types.
* The above will allow those protocols, plus Encodable, Decodable, typealias Codable, Encoder, Decoder, CodingKey, struct CodingUserInfoKey to be part of the standard library (not in Foundation), resolving the concern about reaching too far up the stack for the compiler.
* JSONEncoder/Decoder, PropertyListEncoder/Decoder remain in Foundation. These are concrete implementations of the above protocols, like the ones that will appear in many libraries, we hope.

We are not currently proposing using the integer protocols. The reasoning is that using them them confers a requirement upon all possible encoders and decoders to support arbitrary-width integers and floating point values. We’re not convinced this is feasible, but we will continue to evaluate.

If the core team accepts the modified proposal, we will get this merged as soon as possible with the goal of allowing all of you to try everything out via Swift’s frequent toolchain snapshots. We will be looking for feedback on how it works in more real world scenarios, and we will consider further adjustments to the API before Swift 4 is final. We are hoping that this extended trial period will allow us to make sure that everything works out the way we expected.

Thanks,
- Tony


(Russ Bishop) #16

* String and Int keys being optional, giving a CodingKey the opportunity to return nil for both at runtime.

This was true in the first public draft, but I believe in this version `stringValue` is no longer Optional.

You are correct; I still don't like the fact that you can be in an unkeyed archiver situation and hitting CodingKey adopters that don't provide integer keys.

* Encoder has three functions, but only one may ever be called. This API is the opposite of "pit of success".

Personally, I'm worried about that too. But I had a lot of trouble coming up with an alternative that didn't violate a more important goal, like "being able to throw errors" or "being compatible with classes".

Last-ditch suggestion: Change a bunch of names so that, for instance, `Encoder` is instead `EncodingContainerizer`. (That's a terrible name, but "Factory" gives me the hives.) That way, the name of the type gives you a hint that you're supposed to use it to make a container. You might even rename the methods to e.g. `useContainer(keyedBy:)`, which sound a little more stateful and mutually-exclusive.

  func encode(to encoder: EncodingContainerizer) throws {
    var container = encoder.useContainer(keyedBy: CodingKeys.self)
    try container.encode(name, forKey: .name)
    try container.encode(birthDate, forKey: .birthDate)
  }

We could just completely separate the idea of keyed and unkeyed encoding?

You may be right that the downsides make an alternative design untenable.

I don't understand why KeyedEncodingContainer needs all those overloads; automatic conformance to Encodable should be provided for the stdlib types in those overloads so why would they be necessary?

I argued about this pretty vigorously. They want to avoid the overhead of building an encoder and single-value container and then making several generic calls to encode the value into the container. Personally, I think this is premature optimization of the highest order—particularly since building an encoder and single-value container are often no-ops—but this is the design they chose, and I don't think it's worth rejecting for that alone.

Has someone done performance tests to validate that this is an issue? If nothing else this seems like an implementation detail a conforming type could opt to provide but it shouldn't be required in every implementation.

I really strongly dislike mixing up the Unkeyed and Keyed concepts. A type should need to explicitly opt-in to supporting unkeyed and that should be enforced at compile time. Unkeyed encoding is a potential versioning nightmare and should be handled with care.

I think they've done a reasonable job of putting unkeyed coding in a sharps drawer by making you specifically ask for it and giving it an ugly name.

Can you clarify what you mean? I see keyed and unkeyed spread across various protocols and types in the proposal. It's front and center.

C#'s serialization attributes are a better and more comprehensive solution but we don't have custom attributes in Swift and property behaviors were deferred. This problem is too important to leave to the future though. If we did ever add custom attributes or if property behaviors get implemented then this design could adopt them incrementally without breaking compatibility (e.g. a serialization transformer behavior that turns a non-Encodable property into an Encodable one, or a behavior that ignores a property for serialization purposes).

On the contrary, I think we *can* safely leave this to the future. If a future version of Swift and Foundation added:

  @uncoded var foo: Bar

That would be a completely additive change. Until then, there's an irritating but serviceable solution available—write your own CodingKeys enum and let code generation write the `Codable` conformances based on it.

I didn't write that sentence clearly; I was aiming for your meaning that we can leave it for the future.

Russ

···

On Apr 12, 2017, at 5:44 PM, Brent Royal-Gordon <brent@architechies.com> wrote:

On Apr 12, 2017, at 11:44 AM, Russ Bishop via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:


(Brent Royal-Gordon) #17

Cosigned. Reflection is really cool, but it's generally going to be the least-efficient, most-buggy possible way to implement a given feature. There's something to be said for just...writing some code. Especially when the computer is writing it for you.

···

On Apr 12, 2017, at 2:12 PM, Zach Waldowski via swift-evolution <swift-evolution@swift.org> wrote:

I want to disagree with this is strongly as possible lest it influence
the proposal in any way whatsoever. Just because you can solve something
through reflection doesn't mean you should.

--
Brent Royal-Gordon
Architechies


(Tony Parker) #18

Hi Brent,

···

On Apr 20, 2017, at 11:37 PM, Brent Royal-Gordon <brent@architechies.com> wrote:

On Apr 20, 2017, at 10:08 AM, Tony Parker via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

* The above will allow those protocols, plus Encodable, Decodable, typealias Codable, Encoder, Decoder, CodingKey, struct CodingUserInfoKey to be part of the standard library (not in Foundation), resolving the concern about reaching too far up the stack for the compiler.

I think this is a huge win for Swift and a great move, but...

* KeyedEncoderContainerProtocol, KeyedDecodingContainerProtocol, UnkeyedEncodingContainer, UnkeyedDecodingContainer, SingleValueEncodingContainer, SingleValueDecodingContainer will drop their Data-taking functions. Data will conform to Codable, so it just goes through the normal paths like other types.

...I hope the `Data`-taking primitive will not be deleted, but rather replaced by one that takes `UnsafeRawBufferPointer` or some similar standard library representation of a byte sequence. Even if it has to be called `encodeBytes` and `decodeBytes` to clarify that it's not somehow coding a pointer, I think a series of bytes truly *is* a primitive type, and something fairly distinct from an unkeyed container of `UInt8`s.

--
Brent Royal-Gordon
Architechies

We still believe the right representation for the majority of data use cases is Data. UnsafeRawBufferPointer is not really the same thing, because (a) it’s unsafe, and (b) it has different mutation characteristics than CoW Data. In any case, this change encourages each encoder to decide for itself how to handle specific types if they make sense for that format. For example, JSONEncoder will switch on the type of the Encodable thing passed to it and do something different if it’s Data. It puts Data in the same bucket as things like Date and URL as types that some coders may just want to treat specially.

It may be reasonable for the *BufferPointer APIs to adopt Codable themselves now. They could encode themselves as [UInt8] by default. Concrete encoders (like JSONEncoder) could switch on the type to do something special if they want, like base64 encoding them. We could easily propose that as an addition to this if we think it’s valuable.

One more thing, which we realized after I sent my original email: the default implementation of many of the protocols needs to throw errors. Therefore we will add enum EncodingError and enum DecodingError to the list of new types. Those enums will have various associated values according to what is useful debug information. To preserve the ability for developers to present these errors to users with localized and user-presentable messages, when these enums are cast to NSError (e as? NSError), they will have the Cocoa error domain and a Foundation-provided code. (This is done via an extension to the enum in Foundation).

- Tony


(Tony Parker) #19

Hi Karl,

Hi everyone,

Thanks for your feedback on this proposal. Based on that plus additional feedback from core team members and others who responded off-thread, we are making the following small adjustments:

* KeyedEncoderContainerProtocol, KeyedDecodingContainerProtocol, UnkeyedEncodingContainer, UnkeyedDecodingContainer, SingleValueEncodingContainer, SingleValueDecodingContainer will drop their Data-taking functions. Data will conform to Codable, so it just goes through the normal paths like other types.
* The above will allow those protocols, plus Encodable, Decodable, typealias Codable, Encoder, Decoder, CodingKey, struct CodingUserInfoKey to be part of the standard library (not in Foundation), resolving the concern about reaching too far up the stack for the compiler.
* JSONEncoder/Decoder, PropertyListEncoder/Decoder remain in Foundation. These are concrete implementations of the above protocols, like the ones that will appear in many libraries, we hope.

We are not currently proposing using the integer protocols. The reasoning is that using them them confers a requirement upon all possible encoders and decoders to support arbitrary-width integers and floating point values. We’re not convinced this is feasible, but we will continue to evaluate.

If the core team accepts the modified proposal, we will get this merged as soon as possible with the goal of allowing all of you to try everything out via Swift’s frequent toolchain snapshots. We will be looking for feedback on how it works in more real world scenarios, and we will consider further adjustments to the API before Swift 4 is final. We are hoping that this extended trial period will allow us to make sure that everything works out the way we expected.

Thanks,
- Tony

That’s a great step; thanks a lot!

As for Data/UnsafeRawBufferPointer (beyond this proposal): I agree that a Data object which owns its buffer should be the common-currency for passing buffers around, but I also think that the standard library should own that “Data” type. Data is (IMO), no less "primitive” than, say, Array, Dictionary, Set and String - yet while all of those types are part of the standard library, Data lives much further down the stack, in Foundation. The standard library is limited to unsafe buffers, without a “safe” alternative. Even though Foundation is a core library and very useful, it is still an optional component of any Swift installation or application, and I feel it’s not unreasonable to ask for a safe buffer without that dependency.

I could of course define my own RAII, UnsafeRawBufferPointer-wrapping “Data” type, but then it either spreads throughout my API and undermines Data as a common currency, or I need to convert it (with a copy, in order to preserve value semantics). This is the same issue that affects DispatchData/Data bridging, which in that case undermines APIs such as DispatchIO; either all of my APIs now take a DispatchData (leaking implementation details), or I have to copy the buffer in to a Foundation Data.

We’re aware of this, but it’s something I’d like to tackle separately from the Coding proposals.

That’s an issue which I would like to help solve more comprehensively one day, but for the mean time:

1) Is untyped data really a “primitive” in the context of serialisation? If so...
2) Why are we removing it? Is it only because the standard library lacks a good type to represent the data? If so...
3) Why not introduce a generic function?

mutating func encodeBytes<C>(_ bytes: C, forKey key: Key) throws where C: Collection, C.Element == UInt8

This single function would be able to encode Data, DispatchData, UnsafeRawBufferPointer, and even an Array<UInt8>.

- Karl

We considered something along these lines:

    func encode<T : Encodable>(_ value: T?, ...) // existing
    func encode<T : RangeRepleaceableCollection>(_ value: T?, ...) where T.Iterator.Element == UInt8

but this is ambiguous when we have conditional conformance and Array where Element : Encodable is Encodable. Did you mean to encode base64-encoded data for JSON or an array of UInt8? Data resolves the ambiguity by assigning more meaning to a Collection of UInt8.

We could resolve the ambiguity by using a different name (like you suggest above), but this reduces the viability of Data as a common currency type, which we believe is very important. Ultimately we felt that going with one less primitive here was a better approach.

- Tony

···

On Apr 22, 2017, at 6:23 AM, Karl Wagner <razielim@gmail.com> wrote:

On 20 Apr 2017, at 19:08, Tony Parker via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:


(Brent Royal-Gordon) #20

* String and Int keys being optional, giving a CodingKey the opportunity to return nil for both at runtime.

This was true in the first public draft, but I believe in this version `stringValue` is no longer Optional.

You are correct; I still don't like the fact that you can be in an unkeyed archiver situation and hitting CodingKey adopters that don't provide integer keys.

You are building this dislike on top of several misconceptions:

* There are no "keyed archivers" and "unkeyed archivers". There are keyed and unkeyed *containers*, and all coders handle both kinds.

* Unkeyed containers don't use keys at all, so this has nothing to do with keyed vs. unkeyed. Unkeyed containers are for storing a series of values with no inherent structure to them, only an order. Array elements and tuples are good examples. (But of course, you'd just encode the array as a whole into an entry in a keyed container and the array itself would create an unkeyed container and put its elements into it.)

* Integer keys are a space-efficient alternative to identifying keys by string value; they give supporting coders a fixed-width value they can use to represent a key. But they don't need to be contiguous and they don't necessarily have any influence on the order that values are encoded in. As long as you don't change an existing field's integerValue or reuse an integerValue previously assigned to a different key, there should be no versioning problems.

* Encoder has three functions, but only one may ever be called. This API is the opposite of "pit of success".

Personally, I'm worried about that too. But I had a lot of trouble coming up with an alternative that didn't violate a more important goal, like "being able to throw errors" or "being compatible with classes".

Last-ditch suggestion: Change a bunch of names so that, for instance, `Encoder` is instead `EncodingContainerizer`. (That's a terrible name, but "Factory" gives me the hives.) That way, the name of the type gives you a hint that you're supposed to use it to make a container. You might even rename the methods to e.g. `useContainer(keyedBy:)`, which sound a little more stateful and mutually-exclusive.

  func encode(to encoder: EncodingContainerizer) throws {
    var container = encoder.useContainer(keyedBy: CodingKeys.self)
    try container.encode(name, forKey: .name)
    try container.encode(birthDate, forKey: .birthDate)
  }

We could just completely separate the idea of keyed and unkeyed encoding?

As I said, I think you misunderstand what these things are for.

I would like, though, to have the type declare the kind of container it wants. In a previous thread, I suggested that `encode(to:)` should take a container directly, and we should construct it for them:

  func encode(to container: KeyedEncodingContainer<CodingKeys>) throws {
    try container.encode(name, forKey: .name)
    try container.encode(birthDate, forKey: .birthDate)
  }

The authors pointed out that this would require you to make the `CodingKeys` type public and would mean that subclasses couldn't switch container types. These really are important problems with any idea of that sort.

I argued about this pretty vigorously. They want to avoid the overhead of building an encoder and single-value container and then making several generic calls to encode the value into the container. Personally, I think this is premature optimization of the highest order—particularly since building an encoder and single-value container are often no-ops—but this is the design they chose, and I don't think it's worth rejecting for that alone.

Has someone done performance tests to validate that this is an issue? If nothing else this seems like an implementation detail a conforming type could opt to provide but it shouldn't be required in every implementation.

I don't know what testing they've done, but this is not something that individual coders could add after the fact. `encode(to:)` and `init(from:)` are passed only protocol existentials containing the coder, so they don't have access to any member that isn't in the protocol. The same for containers. Accessing any special, coder-specific methods would perhaps not be impossible, but it'd be prohibitively difficult. So if any coders want to have this fast path, it needs to be part of the protocol ahead of time; it can't be bolted on by only the conforming types that want it.

I think they've done a reasonable job of putting unkeyed coding in a sharps drawer by making you specifically ask for it and giving it an ugly name.

Can you clarify what you mean? I see keyed and unkeyed spread across various protocols and types in the proposal. It's front and center.

What I mean is that, when you get an encoder, you have three choices: `container(keyedBy:)`, `unkeyedContainer()`, and `singleValueContainer()`. If you looked into single-value, you'd quickly discover that it only supports a few low-level types (actually, I think it might be better to call it `singlePrimitiveValueContainer()`. So that leaves you with keyed and unkeyed. `unkeyedContainer()` has an ugly, awkward name (this is actually intentional), and just about every piece of documentation or tutorial is going to tell you not to use it. Meanwhile, `container(keyedBy:)` is nice and we generate all sorts of code and stuff to make it easy to use. Once you're into a keyed container, there's no way to accidentally use any unkeyed methods.

And all of this is only an issue if you write the conformance by hand in the first place. Usually, you won't.

···

On Apr 12, 2017, at 6:18 PM, Russ Bishop <rbishopjr@apple.com> wrote:

On Apr 12, 2017, at 5:44 PM, Brent Royal-Gordon <brent@architechies.com <mailto:brent@architechies.com>> wrote:

On Apr 12, 2017, at 11:44 AM, Russ Bishop via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

--
Brent Royal-Gordon
Architechies