{"action":"created","issue":{"url":"https://api.github.com/repos/apple/swift/issues/5521","repository_url":"https://api.github.com/repos/apple/swift","labels_url":"https://api.github.com/repos/apple/swift/issues/5521/labels{/name}","comments_url":"https://api.github.com/repos/apple/swift/issues/5521/comments","events_url":"https://api.github.com/repos/apple/swift/issues/5521/events","html_url":"https://github.com/apple/swift/pull/5521","id":186056960,"number":5521,"title":"[stdlib] [WIP] Eliminate version of Array.append(contentsOf:) that takes a Collection","user":{"login":"airspeedswift","id":8058277,"avatar_url":"https://avatars.githubusercontent.com/u/8058277?v=3","gravatar_id":"","url":"https://api.github.com/users/airspeedswift","html_url":"https://github.com/airspeedswift","followers_url":"https://api.github.com/users/airspeedswift/followers","following_url":"https://api.github.com/users/airspeedswift/following{/other_user}","gists_url":"https://api.github.com/users/airspeedswift/gists{/gist_id}","starred_url":"https://api.github.com/users/airspeedswift/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/airspeedswift/subscriptions","organizations_url":"https://api.github.com/users/airspeedswift/orgs","repos_url":"https://api.github.com/users/airspeedswift/repos","events_url":"https://api.github.com/users/airspeedswift/events{/privacy}","received_events_url":"https://api.github.com/users/airspeedswift/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":35,"created_at":"2016-10-29T03:36:27Z","updated_at":"2016-11-05T00:21:45Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/apple/swift/pulls/5521","html_url":"https://github.com/apple/swift/pull/5521","diff_url":"https://github.com/apple/swift/pull/5521.diff","patch_url":"https://github.com/apple/swift/pull/5521.patch"},"body":"Resolves ABI FIXMEs #13, #16 and #17. Changes `Sequence._copyContents` to take a `UnsafeMutableBufferPointer` instead of just a pointer, adds checks to ensure it doesn't initialize memory outside that buffer in the default implementation, and optionally returns an `Iterator` with any unwritten elements.\r\n\r\nCurrently there are two versions of `Array.append(contentsOf:)`, one that takes a sequence and one that takes a collection. We want to collapse them into one, and then move the definition of `+=` up into `RangeReplaceableCollection` (which doesn't have a version of `append` specialized for collections). At the same time, there is a potentially nasty bug that impacts the collection version. The solution for both problems is similar.\r\n\r\nIn theory the collection version is optimized, because we can determine in advance how many elements are in the collection being appended. The current version allocates enough memory, then calls a method on its buffer's unsafe pointer to initialize from the collection. **This is bad** because collections can lie about their count – for example, a reference-semantic collection like `NSMutableArray` might change on another thread while copying, or this code which uses external state to make a lazy collection give different results:\r\n\r\n``` swift\r\nvar i = 0\r\nlet l = repeatElement(42, count: 10_000).lazy.filter { _ in i += 1; return i > 10_000 }\r\nvar a: [Int] = []\r\na.append(contentsOf: l) // goes bang\r\n```\r\n\r\nIf a collection underreports its count, the current code will end up scribbling over the memory past the end of the buffer.\r\n\r\nTo do this safely, what we need to do is take the count, reserve enough memory, then use an `UnsafeBufferPointer` to append safely not beyond the count. Then any remaining elements that weren't accounted for by the collection's count can be appended too. These remaining elements are returned as a partially consumed iterator by the append operation.\r\n\r\nThis solution will also allow us to combine with the version for `Sequence`, which basically needs the same thing, only with a call to `underestimatedCount` since this can also be an underestimate (non-zero underestimates might not be the total number of elements in the sequence).\r\n\r\nCollections that are certain they have value semantics and never lie about their counts can choose to override the method that the unsafe pointer calls on them to write themselves into memory, to skip certain safety checks the default implementation puts in.\r\n"},"comment":{"url":"https://api.github.com/repos/apple/swift/issues/comments/258577580","html_url":"https://github.com/apple/swift/pull/5521#issuecomment-258577580","issue_url":"https://api.github.com/repos/apple/swift/issues/5521","id":258577580,"user":{"login":"airspeedswift","id":8058277,"avatar_url":"https://avatars.githubusercontent.com/u/8058277?v=3","gravatar_id":"","url":"https://api.github.com/users/airspeedswift","html_url":"https://github.com/airspeedswift","followers_url":"https://api.github.com/users/airspeedswift/followers","following_url":"https://api.github.com/users/airspeedswift/following{/other_user}","gists_url":"https://api.github.com/users/airspeedswift/gists{/gist_id}","starred_url":"https://api.github.com/users/airspeedswift/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/airspeedswift/subscriptions","organizations_url":"https://api.github.com/users/airspeedswift/orgs","repos_url":"https://api.github.com/users/airspeedswift/repos","events_url":"https://api.github.com/users/airspeedswift/events{/privacy}","received_events_url":"https://api.github.com/users/airspeedswift/received_events","type":"User","site_admin":false},"created_at":"2016-11-05T00:21:45Z","updated_at":"2016-11-05T00:21:45Z","body":"Hmm, switching to that method had a significant performance reduction, especially unoptimized."}}
{"actor":{"display_login":"airspeedswift"}}