Swift Archival & Serialization API #9004

itaiferber · 2017-04-25T20:19:51Z

What's in this pull request?
Now that the core types introduced by SE-0166 have been sunk into the Swift standard library, the stdlib and compiler portions of #8124 and #8125 can be combined into one PR (this one).

The features can be tested together; #8125 was previously not tested on its own since it required the presence of #8124.

The Foundation portions of #8124 will be put up in a subsequent PR.

rdar://problem/30472044

Add Encodable and Decodable protocols, along with associated container types, default implementations, and extensions to the standard library

* Allow CodingKey conformance to be automatically derived for enums which have no raw type (with no associated values) and which have a raw type of String or Int. * Allow Encodable and Decodable conformance to be automatically derived for classes and structs with Encodable/Decodable properties * Add initial unit tests for verifying derived conformance

Since all of the core types are now in the Swift stdlib, the tests no longer rely on objc_interop or Foundation.

itaiferber · 2017-04-26T16:02:47Z

@swift-ci Please test

swift-ci · 2017-04-26T17:16:51Z

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 22185d2
Test requested by - @itaiferber

swift-ci · 2017-04-26T17:46:02Z

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 22185d2
Test requested by - @itaiferber

itaiferber · 2017-04-26T17:49:31Z

I'm not sure these test failures are actually related to my changes. On Linux we're getting failures like error: 'Iterator' is not a member type of 'Sequence', and on macOS, the failing tests are SourceKit. The Linux failures are more worrisome to me though; anyone know what's going on there?

itaiferber · 2017-04-26T21:28:47Z

Let's try again

itaiferber · 2017-04-26T21:28:52Z

@swift-ci Please test

swift-ci · 2017-04-26T21:30:58Z

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 22185d2
Test requested by - @itaiferber

swift-ci · 2017-04-26T21:41:50Z

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 22185d2
Test requested by - @itaiferber

igorcamilo · 2017-04-26T22:48:19Z

stdlib/public/core/Codable.swift

+    /// - throws: `DecodingError.typeMismatch` if the encountered encoded value is not convertible to the requested type.
+    /// - throws: `DecodingError.keyNotFound` if `self` does not have an entry for the given key.
+    /// - throws: `DecodingError.valueNotFound` if `self` has a null entry for the given key.
+    func decode<T : Decodable>(_ type: T.Type, forKey key: Key) throws -> T


@itaiferber great proposal, can't wait to start using this. But I have a question: wouldn't be better to drop the type from the parameters and just use inference here? Something like:

func decode<T : Decodable>(_ key: Key) throws -> T

@Obitow Thanks!

This point has been discussed at length on swift-evolution; dropping the type parameter can lead to ambiguous statements, which can be confusing and unintuitive. Please feel free to refer to those discussions for the full story.

itaiferber · 2017-04-26T23:01:13Z

@atrick @gottesmm Looks like the tests are failing in SIL validation on Linux and the errors are kind of bizarre: e.g. error: 'Iterator' is not a member type of 'Sequence'. Any idea about what's going on here/how I can address this?

itaiferber · 2017-04-26T23:08:59Z

Perhaps we can try the smoke tests at least?

itaiferber · 2017-04-26T23:09:05Z

@swift-ci Please smoke test

itaiferber · 2017-04-27T17:22:54Z

It's possible that Sequence : Swift.Sequence was causing ambiguity when generating SIL. I've changed that; let's see if that's caused any changes in the tests. Once more with vigor...

itaiferber · 2017-04-27T17:23:00Z

@swift-ci Please test

swift-ci · 2017-04-27T17:37:31Z

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 53d82b1
Test requested by - @itaiferber

itaiferber · 2017-04-27T17:38:52Z

Looks like the Sequence change did fix the SIL tests. The SourceKit tests are still failing though.

swift-ci · 2017-04-27T17:43:54Z

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 53d82b1
Test requested by - @itaiferber

struct X : Codable {} should generate an encode(to:) which encodes an empty keyed container, not an encode(to:) which does nothing

Extensions need not have nominal types, e.g. extension Int : Codable { ... } where typealias Codable = Encodable & Decodable

itaiferber · 2017-04-27T19:58:38Z

@swift-ci Please test

swift-ci · 2017-04-27T19:58:55Z

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 53d82b1
Test requested by - @itaiferber

DougGregor · 2017-04-28T17:49:45Z

It seems like a bug like this in the actual AST (e.g., duplicated initializers) would cause everything to break horribly, so it feels like a SourceKit bug.

You shouldn't need to be Codable in order to get these implementations; being either Encodable or Decodable should be enough to get the relevant default implementation

itaiferber · 2017-04-28T18:16:01Z

This last commit doesn't fix the issue, but if I'm already looking at the extensions... (Let's split them up so you can benefit even by just being Encodable or Decodable.) Still waiting to hear back on SourceKit though.

itaiferber · 2017-04-28T18:17:09Z

stdlib/public/core/Codable.swift

+    ///
+    /// - parameter lhs: The key to compare against.
+    /// - parameter rhs: The key to compare with.
+    public static func ==(_ lhs: CodingUserInfoKey, _ rhs: CodingUserInfoKey) -> Bool {


I'd meant to split this out into a different commit, but it made it in here anyway. CodingUserInfoKey was missing ==, but the compiler didn't complain.

itaiferber · 2017-04-28T20:34:03Z

Per @akyrtzi's suggestion via email, going to update the test to prevent this from blocking us and file a Radar about it.

itaiferber · 2017-04-28T20:38:22Z

@swift-ci Please test

swift-ci · 2017-04-28T20:38:41Z

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 4c23207
Test requested by - @itaiferber

swift-ci · 2017-04-28T20:40:32Z

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 4c23207
Test requested by - @itaiferber

itaiferber · 2017-04-28T21:00:52Z

Build failed with an unrelated issue that's been reverted; let's try again.

itaiferber · 2017-04-28T21:00:59Z

@swift-ci Please test

itaiferber · 2017-04-28T22:00:16Z

For interested Apple folks, BTW, rdar://31896858

swift-ci · 2017-04-28T22:03:51Z

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 433c192
Test requested by - @itaiferber

itaiferber · 2017-04-28T22:08:33Z

Looks like a Migrator test is failing for the iOS simulator: Migrator/wrap_optional.swift
Looking into it.

itaiferber · 2017-04-28T22:10:19Z

The builder is producing a module file for the incorrect target? error: module file was created for incompatible target x86_64-apple-macosx10.9

itaiferber · 2017-04-28T22:11:36Z

@shahmishal @DougGregor This... Seems wrong. Any idea why this might happen?

itaiferber · 2017-04-28T22:28:18Z

Seems like https://ci.swift.org/view/Pull%20Request/job/swift-PR-osx/ failed with this as well...

itaiferber · 2017-04-28T23:10:44Z

Waiting on #9113 before testing again.

itaiferber · 2017-04-28T23:49:09Z

Let's try that again

itaiferber · 2017-04-28T23:49:14Z

@swift-ci Please test

itaiferber · 2017-04-29T02:41:20Z

Excellent. @parkera @DougGregor If either of you sees this, can you please merge it in?

itaiferber · 2017-04-29T03:03:34Z

Thanks @DougGregor!

norio-nomura · 2017-05-10T04:03:04Z

stdlib/public/core/Codable.swift

+
+// An implementation of _KeyedEncodingContainerBase and _KeyedEncodingContainerBox are given at the bottom of this file.
+/// `KeyedEncodingContainer` is a type-erased box for `KeyedEncodingContainerProtocol` types, similar to `AnyCollection` and `AnyHashable`. This is the type which consumers of the API interact with directly.
+public struct KeyedEncodingContainer<K : CodingKey> {


@itaiferber Should KeyedEncodingContainer declare KeyedEncodingContainerProtocol conformance?

It can, but I don't see much of a benefit to doing so. When would you want to use it as KeyedEncodingContainerProtocol instead of the fully-typed KeyedEncodingContainer<K>?

Indeed, you are right. I can not think of such a scene.
I just wondered watching KeyedDecodingContainer declared asKeyedDecodingContainerProtocol.

Good catch — I don't think that's really necessary either. It doesn't hurt, but we should be consistent. I'll consider this; thanks!

norio-nomura · 2017-05-10T04:12:33Z

stdlib/public/core/Codable.swift

+
+    /// Decodes a value of the given type for the given key, if present.
+    ///
+    /// This method retu


@itaiferber Why is it mutating only for decode() of UnkeyedDecodingContainer?

UnkeyedDecodingContainers need to maintain some form of state to keep track of where they are in decoding, whether an index into an array that's incrementing with every decode(...) call, an offset into a data blob or stream, etc. The methods are mutating to indicate that, and to allow the implementation to be a struct. This is as opposed to KeyedDecodingContainers and SingleValueDecodingContainers, which by design will almost certainly not need to do that. Is there a use case you have in mind for making those mutating as well?

Certainly, I could not think of any reason to make decode() of another protocol mutating as well.
Thank you for answering my boring questions. 🙏

Not at all! Thank you for paying such close attention to this; it's the best way for us to get direct feedback on it, and I appreciate your efforts. 😄

norio-nomura · 2017-05-11T13:13:26Z

I filed SR-4861 that request adding encode<T>(_:) and decode<T>(_:) to SingleValueEncodingContainer and SingleValueDecodingContainer.

itaiferber · 2017-05-11T15:59:35Z

@norio-nomura Thanks, assigned that to myself. That change, among a few others, is planned, and going through some API review soon.

Jceciliani

All committed changes appear correct after review.

Jceciliani · 2018-05-21T06:09:32Z

test/decl/protocol/special/coding/class_codable_inheritance.swift

+    let _ = SimpleChildClass.CodingKeys.self // expected-error {{'CodingKeys' is inaccessible due to 'private' protection level}}
+
+    // The enum should have a case for each of the vars.
+    // NOTE: This expectedxerror will need to be removed in the future.


Small text fix needed in comment. "//NOTE: This expected error will need to be removed in the future."

Thanks! #16749

Itai Ferber added 2 commits April 25, 2017 13:04

Integrate *Codable protocols and associated types

cecf00d

Add Encodable and Decodable protocols, along with associated container types, default implementations, and extensions to the standard library

This was referenced Apr 25, 2017

Explicit Codable & CodingKey Derived Conformance #8125

Closed

Swift Archival & Serialization API #8124

Closed

Foundation Encoders #9005

Merged

CodingKey/*Codable tests don't need ObjC interop

22185d2

Since all of the core types are now in the Swift stdlib, the tests no longer rely on objc_interop or Foundation.

Remove unnecessary enum case check

c515b53

igorcamilo reviewed Apr 26, 2017

View reviewed changes

Rename sequence type to avoid ambiguity

53d82b1

Itai Ferber added 3 commits April 27, 2017 10:56

Derive encoding an empty container by default

3e6eb8f

struct X : Codable {} should generate an encode(to:) which encodes an empty keyed container, not an encode(to:) which does nothing

Fix crash in AST printing

bdfa927

Extensions need not have nominal types, e.g. extension Int : Codable { ... } where typealias Codable = Encodable & Decodable

Fix init(from:) required condition

4c23207

Split RawRep extensions for Encodable + Decodable

e1e6784

You shouldn't need to be Codable in order to get these implementations; being either Encodable or Decodable should be enough to get the relevant default implementation

itaiferber commented Apr 28, 2017

View reviewed changes

Temporarily fix failing SourceKit test

433c192

DougGregor merged commit 447dce6 into swiftlang:master Apr 29, 2017

itaiferber deleted the swift-archival-serialization branch April 29, 2017 03:03

norio-nomura reviewed May 10, 2017

View reviewed changes

noahsark769 mentioned this pull request Jan 18, 2018

Fix a comment typo in DerivedConformances #14011

Merged

Jceciliani reviewed May 21, 2018

View reviewed changes

Swift Archival & Serialization API #9004

Swift Archival & Serialization API #9004

Conversation

itaiferber commented Apr 25, 2017 • edited by najacque Loading

itaiferber commented Apr 26, 2017

swift-ci commented Apr 26, 2017

swift-ci commented Apr 26, 2017

itaiferber commented Apr 26, 2017

itaiferber commented Apr 26, 2017

itaiferber commented Apr 26, 2017

swift-ci commented Apr 26, 2017

swift-ci commented Apr 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itaiferber commented Apr 26, 2017

itaiferber commented Apr 26, 2017

itaiferber commented Apr 26, 2017

itaiferber commented Apr 27, 2017

itaiferber commented Apr 27, 2017

swift-ci commented Apr 27, 2017

itaiferber commented Apr 27, 2017

swift-ci commented Apr 27, 2017

itaiferber commented Apr 27, 2017

swift-ci commented Apr 27, 2017

DougGregor commented Apr 28, 2017

itaiferber commented Apr 28, 2017 • edited Loading

Choose a reason for hiding this comment

itaiferber commented Apr 28, 2017

itaiferber commented Apr 28, 2017

swift-ci commented Apr 28, 2017

swift-ci commented Apr 28, 2017

itaiferber commented Apr 28, 2017

itaiferber commented Apr 28, 2017

itaiferber commented Apr 28, 2017

swift-ci commented Apr 28, 2017

itaiferber commented Apr 28, 2017 • edited Loading

itaiferber commented Apr 28, 2017

itaiferber commented Apr 28, 2017

itaiferber commented Apr 28, 2017

itaiferber commented Apr 28, 2017

itaiferber commented Apr 28, 2017

itaiferber commented Apr 28, 2017

itaiferber commented Apr 29, 2017 • edited Loading

itaiferber commented Apr 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

norio-nomura May 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itaiferber May 11, 2017 • edited Loading

Choose a reason for hiding this comment

norio-nomura commented May 11, 2017

itaiferber commented May 11, 2017

Jceciliani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itaiferber commented Apr 25, 2017 •

edited by najacque

Loading

itaiferber commented Apr 28, 2017 •

edited

Loading

itaiferber commented Apr 28, 2017 •

edited

Loading

itaiferber commented Apr 29, 2017 •

edited

Loading

norio-nomura May 10, 2017 •

edited

Loading

itaiferber May 11, 2017 •

edited

Loading