[swift-evolution] Possible issue with SE-0166 Swift Archival & Serialization implementation

[Brent Royal-Gordon]

> On Jun 8, 2017, at 8:27 AM, Gwendal Roué via swift-evolution <swift-evolution at swift.org> wrote:
>> 
>> Le 8 juin 2017 à 16:51, James Froggatt via swift-evolution <swift-evolution at swift.org> a écrit :
>> 
>> Here, container is stored as a `let` value, and uses reference semantics, while the proposal also clearly lists these `encode` methods as mutating. With the current implementation of the proposal, the container must be stored as a `var`, which leads to code like the following:
>> 
>>   var container = encoder.singleValueContainer()
>>   try container.encode(data)
> 
> Yes, practically speaking and with latest Swift 4, the container needs to be declared as `var`.
> 
> I admit it's weird, and feels unnatural:
> 
>   public func encode(to encoder: Encoder) throws {
>       // A mutated value that nobody consumes: so weird.
>       var container = encoder.container(keyedBy: CodingKeys.self)
>       try container.encode(latitude, forKey: .latitude)
>       try container.encode(longitude, forKey: .longitude)
>   }
> 
>> This clearly wont work as expected if the container were to have value semantics, and writing code like this feels plain wrong. Is SE-0166 really intended to work with referrence-type encoders only?
> 
> Actually, it can work with encoder/containers that have value semantics, and forward the mutations somewhere else (for example to a closure which fills a mutating container).
> 
> But this is again bizarre, and contrieved: https://github.com/groue/GRDB.swift/blob/15bfe5f6cf76070cfb17216223bdebc6b158d654/GRDB/Record/Persistable%2BEncodable.swift <https://github.com/groue/GRDB.swift/blob/15bfe5f6cf76070cfb17216223bdebc6b158d654/GRDB/Record/Persistable%2BEncodable.swift>
> 
> You make me think that those structs should swiftly be refactored into reference types.


I made precisely this comment during the review. The response was that the container could potentially defined by a struct that contains some state which it uses to write into a reference. For instance, imagine a type like this:

	class MyEncoder: Encoder {
		var buffer: Data
		
		struct KeyedEncodingContainer<K: CodingKey>: KeyedEncodingContainerProtocol {
			let encoder: MyEncoder
			var bufferOffset: Int
			
			mutating func encode(_ value: String, forKey key: K) throws {
				let newData = try makeDataFrom(value, forKey: key)
				
				encoder.buffer.insert(newData, at: offset)
				bufferOffset += newData.count
			}
			…
		}
		…
	}

I argued that foreclosing designs like this would be acceptable—you could either make `KeyedEncodingContainer` a class or move `offset` into `MyEncoder` to get around it—but the proposers preferred to preserve this flexibility.

-- 
Brent Royal-Gordon
Architechies

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170608/36572533/attachment.html>