-
Notifications
You must be signed in to change notification settings - Fork 441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Copy #52
Open
CTMacUser
wants to merge
17
commits into
apple:main
Choose a base branch
from
CTMacUser:copy
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Copy #52
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
e8f7bf3
Add method to copy elements
CTMacUser c007b8d
Add another method to copy elements
CTMacUser 8e19993
Correct name of parameter in documentation
CTMacUser 39021cb
Add methods to copy elements onto a suffix
CTMacUser 440456f
Correct names of tests
CTMacUser f143910
Add method to copy elements backwards
CTMacUser effb1d3
Add methods to internally copy elements
CTMacUser 676de79
Improve documentation; rename methods
CTMacUser b354606
Rename the "copy" methods to "overwrite"
CTMacUser 3a9f943
Redo documentation
CTMacUser d92b9d2
Add methods to copy elements from iterators
CTMacUser 501154c
Simplify copying from a sequence
CTMacUser 788e7a8
Add direction parameter when copying from an iterator
CTMacUser 27b61e2
Replace methods to copy from collections
CTMacUser 50f5855
Update documentation
CTMacUser 79af788
Rename the files from "Copy" to "Overwrite"
CTMacUser 99d436b
Correct spelling error
CTMacUser File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
# Overwrite | ||
|
||
[[Source](../Sources/Algorithms/Overwrite.swift) | | ||
[Tests](../Tests/SwiftAlgorithmsTests/OverwriteTests.swift)] | ||
|
||
Copy a sequence onto an element-mutable collection. | ||
|
||
```swift | ||
var destination = Array("abcde") | ||
print(String(destination)) // "abcde" | ||
|
||
let source = "123" | ||
let changedEnd = destination.overwrite(prefixWith: source) | ||
print(String(destination)) // "123de" | ||
print(String(destination[changedEnd...])) // "de" | ||
``` | ||
|
||
`overwrite(prefixWith:)` takes a source sequence and replaces the first `k` | ||
elements of the receiver with the first `k` elements of the source, where *k* | ||
is the length of the shorter sequence. `overwrite(forwardsWith:)` does the same | ||
thing with a source collection, and `overwrite(prefixUsing:)` with an `inout` | ||
source iterator. To preserve memory exclusivity, the | ||
`overwrite(forwardsFrom:to:)` overload is required to copy between subsequences | ||
of the same collection, where the source and destination are given as index | ||
ranges. | ||
|
||
When the receiving element-mutable collection supports bidirectional traversal, | ||
variants of the previous methods are defined that copy the source elements on | ||
top of the receiver's suffix instead. The `overwrite(suffixWith:)` and | ||
`overwrite(suffixUsing:)` methods use their source's prefix, while the | ||
`overwrite(backwardsWith:)` and `overwrite(backwardsFrom:to:)` methods use | ||
their source's suffix. | ||
|
||
## Detailed Design | ||
|
||
New methods are added to element-mutable collections: | ||
|
||
```swift | ||
extension MutableCollection { | ||
mutating func overwrite<I>(prefixUsing source: inout I) -> Index | ||
where I : IteratorProtocol, Self.Element == I.Element | ||
|
||
mutating func overwrite<S>(prefixWith source: S) -> Index | ||
where S : Sequence, Self.Element == S.Element | ||
|
||
mutating func overwrite<C>(forwardsWith source: C) | ||
-> (readEnd: C.Index, writtenEnd: Index) | ||
where C : Collection, Self.Element == C.Element | ||
|
||
mutating func overwrite<R, S>(forwardsFrom source: R, to destination: S) | ||
-> (sourceRead: Range<Index>, destinationWritten: Range<Index>) | ||
where R : RangeExpression, S : RangeExpression, Self.Index == R.Bound, | ||
R.Bound == S.Bound | ||
} | ||
|
||
extension MutableCollection where Self: BidirectionalCollection { | ||
mutating func overwrite<I>(suffixUsing source: inout I) -> Index | ||
where I : IteratorProtocol, Self.Element == I.Element | ||
|
||
mutating func overwrite<S>(suffixWith source: S) -> Index | ||
where S : Sequence, Self.Element == S.Element | ||
|
||
mutating func overwrite<C>(backwardsWith source: C) | ||
-> (readStart: C.Index, writtenStart: Index) | ||
where C : BidirectionalCollection, Self.Element == C.Element | ||
|
||
mutating func overwrite<R, S>(backwardsFrom source: R, to destination: S) | ||
-> (sourceRead: Range<Index>, destinationWritten: Range<Index>) | ||
where R : RangeExpression, S : RangeExpression, Self.Index == R.Bound, | ||
R.Bound == S.Bound | ||
} | ||
``` | ||
|
||
When the source is an iterator or sequence, the return value from `overwrite` | ||
is a single index value within the receiver that is the non-anchored end of the | ||
range of overwritten elements. The prefix-overwriting methods return the upper | ||
bound, *i.e.* the index after the last touched element, and assume the lower | ||
bound is the receiver's `startIndex`. The suffix-overwriting methods return the | ||
lower bound, *i.e.* the index of the first touched element, and assume the | ||
upper bound is the receiver's `endIndex`. Use of the return value is optional | ||
to support casual use of copying without caring about the precise range of | ||
effect. | ||
|
||
When the source is a collection, the return value from `overwrite` has two | ||
components. The second component is the same as the sole value returned from | ||
the overloads with iterator or sequence sources. The first component is the | ||
non-anchored end of the range of the elements actually read from the source. | ||
When the source is a subsequence, the return value's components are index | ||
ranges fully bounding the touched elements instead of ranges implied from | ||
isolated indices. | ||
|
||
### Complexity | ||
|
||
Calling these methods is O(_k_), where _k_ is the length of the shorter | ||
(virtual) sequence between the receiver (subsequence) and the source. | ||
|
||
### Naming | ||
|
||
The initial development version of this library used the term-of-art "`copy`" | ||
as the base name of this family of methods. But since the insertion-copying | ||
methods (in `RangeReplaceableCollection`) do not use the term, and the term is | ||
used for object copying in Foundation, a substitute term was chosen here. The | ||
term "`overwrite`" gives a precise description of the kind of copying employed. | ||
|
||
### Comparison with other languages | ||
|
||
**C++:** Has a [`copy`][C++Copy] function in the `algorithm` library that takes | ||
a bounding pair of input iterators for the source and a single output iterator | ||
for the destination, returning one-past the last output iterator written over. | ||
The `copy_if` function does not have an analogue, since it can be simulated by | ||
submitting the result from `filter(_:)` as the source. There is a | ||
[`copy_backward`][C++CopyBackward] function that copies elements backwards from | ||
the far end of the source and destination, returning the near end of the | ||
destination that got written. These functions take their buffer arguments as | ||
separate iterator/pointer values; as such, the functions can handle the source | ||
and destination buffers having overlap or otherwise being sub-buffers of a | ||
shared collection. Swift's memory safety model prevents it from doing the | ||
same, necessitating it to use customized methods when the source and | ||
destination buffers subset the same super-buffer. | ||
|
||
<!-- Link references for other languages --> | ||
|
||
[C++Copy]: https://en.cppreference.com/w/cpp/algorithm/copy | ||
[C++CopyBackward]: https://en.cppreference.com/w/cpp/algorithm/copy_backward |
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This indeed seems a nice proposal and well implemented, but I was wondering that this functionality is kind of already covered in a more general way by
RangeReplaceableCollection
'sreplaceSubrange(:with:)
.So those are more questions/suggestions:
What are the advantages of having this API over the existing one?
And also, related to naming, could this complement the existing one by being called
replacePrefix(with:)
?I think this is more align with the convention and easier to reason about given the existing API's available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RangeReplaceableCollection
andMutableCollection
generally can’t be substituted for each other; the API you mentioned is pretty much their sole spiritual overlap, and it wouldn’t work with collections that implement onlyMC
.