[5.3][stdlib] Fix Array.append(contentsOf:) for arguments of type NSArray #34445
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
Due to a couple of unfortunate circumstances, appending an NSArray instance to an Array instance does not actually append any elements. The cause is swiftlang#29220, which accidentally optimized away the actual loop that appends the elements in this particular case. (And only this particular case, which is why this wasn’t detected by the test suite.) When the argument to `Array.append(contentsOf:)` is of type NSArray, the `newElements is [Element]` expression is compiled into a runtime check that returns true, eliminating the subsequent loop over the remaining items of the iterator. Sadly, NSArray.underestimatedCount` currently returns 0, so the earlier _copyContents call is a noop, so no elements get added to `self` at all. Turning the `is` test into a direct equality check between the metatype instances resolves the issue. (cherry picked from commit 184367c)
|
@swift-ci test |
kylemacomber
approved these changes
Oct 26, 2020
|
Windows failure looks unrelated: |
|
@swift-ci test windows platform |
1 similar comment
|
@swift-ci test windows platform |
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 #34426, reviewed by @kylemacomber.
Due to a couple of unfortunate circumstances, appending an NSArray instance to an Array instance does not actually append any elements. The cause is #29220, which accidentally optimized away the actual loop that appends the elements in this particular case. (And only this particular case, which is why this wasn’t detected by the test suite.)
When the argument to
Array.append(contentsOf:)is of type NSArray, thenewElements is [Element]expression is compiled into a runtime check that returns true, eliminating the subsequent loop over the remaining items of the iterator. Sadly,NSArray.underestimatedCountcurrently returns 0, so the earlier_copyContentscall is a noop, so no elements get added toselfat all.Turning the
istest into a direct equality check between the metatype instances resolves the issue.Resolves rdar://70448247.