[5.5][stdlib] Implement _copyContents on internal Array types #37960
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`_copyContents(initializing:)` is a core method of Sequence, and it is used surprisingly often to copy stuff out of sequences. Array’s internal types currently have explicit implementations of it that trap (to prevent a performance bug due to the default iterator-based implementation. This has proved a bad idea, as not all code paths that end up calling `_copyContents` have actually been expunged — so we replaced a performance bug with a catastrophic correctness bug. 😥 Rather than trying to play whack-a-mole with code paths that end up in `_copyContents`, replace the traps with (relatively) efficient implementations, based on the ancient `_copyContents(subRange:initializing)` methods that have already been there all this time. This resolves https://bugs.swift.org/browse/SR-14663. I expect specialization will make this fix deploy back to earlier OSes in most (but unfortunately not all) cases. (cherry picked from commit 466e26a)
|
@swift-ci test |
|
Build failed |
@swift-ci test macOS platform |
|
Build failed |
|
Build failed |
|
Build failed |
(cherry picked from commit 3178ff0)
|
@swift-ci test macOS platform |
|
@swift-ci test linux platform |
|
Build failed |
kylemacomber
approved these changes
Jun 23, 2021
|
@swift-ci test macOS platform |
lorentey
added a commit
to lorentey/swift
that referenced
this pull request
Sep 15, 2021
Welp, the bogus precondition that was fixed in swiftlang#37960 indeed doesn't entirely deploy back to previous OSes. This makes sense -- bridging is generally not specialized. rdar://82125353
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-picked from #37914 & #37968.
_copyContents(initializing:)is a core method of Sequence, and it is used surprisingly often to copy stuff out of sequences. Array’s internal types currently have explicit implementations of it that trap (to prevent a performance bug due to the default iterator-based implementation. This has proved a bad idea, as not all code paths that end up calling_copyContentshave actually been expunged — so we replaced a performance bug with a catastrophic correctness bug. 😥Rather than trying to play whack-a-mole with code paths that end up in
_copyContents, replace the traps with (relatively) efficient implementations, based on the ancient_copyContents(subRange:initializing)methods that have already been there all this time.This resolves https://bugs.swift.org/browse/SR-14663 a.k.a. rdar://78640103.
I expect specialization will make this fix deploy back to earlier OSes in most (but unfortunately not all) cases.